From 41f4564d928c9dba2a953a3889b4232ab30f43e9 Mon Sep 17 00:00:00 2001 From: Grzegorz Nosek Date: Thu, 14 Mar 2024 11:41:19 +0100 Subject: [PATCH] Revert "update(sinsp): implement suppressed tid cache in sinsp_suppress" The cache implementation was broken (a no-op, effectively) but even after fixing it doesn't provide noticeable performance wins. Revert it and save a bit of complexity. This reverts commit 6a9a0ec14d647ad3d73b65f9eb99a33fa1a14eb3. Fixes: #1724 Signed-off-by: Grzegorz Nosek --- userspace/libsinsp/sinsp.cpp | 4 ++-- userspace/libsinsp/sinsp_suppress.cpp | 23 ++++------------------- userspace/libsinsp/sinsp_suppress.h | 10 ++-------- 3 files changed, 8 insertions(+), 29 deletions(-) diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp index fe3362497e..92d2381a58 100644 --- a/userspace/libsinsp/sinsp.cpp +++ b/userspace/libsinsp/sinsp.cpp @@ -1185,7 +1185,7 @@ int32_t sinsp::next(OUT sinsp_evt **puevt) // it from userspace and update the result status if (res == SCAP_SUCCESS) { - res = m_suppress.process_event(evt->get_scap_evt(), evt->get_cpuid()); + res = m_suppress.process_event(evt->get_scap_evt()); } // in case we don't succeed, handle each scenario and return @@ -1426,7 +1426,7 @@ bool sinsp::suppress_events_tid(int64_t tid) bool sinsp::check_suppressed(int64_t tid) const { - return m_suppress.is_suppressed_tid(tid, UINT16_MAX); + return m_suppress.is_suppressed_tid(tid); } void sinsp::set_docker_socket_path(std::string socket_path) diff --git a/userspace/libsinsp/sinsp_suppress.cpp b/userspace/libsinsp/sinsp_suppress.cpp index 79c5561fc1..63c8451f5d 100644 --- a/userspace/libsinsp/sinsp_suppress.cpp +++ b/userspace/libsinsp/sinsp_suppress.cpp @@ -43,7 +43,7 @@ bool libsinsp::sinsp_suppress::check_suppressed_comm(uint64_t tid, const std::st return false; } -int32_t libsinsp::sinsp_suppress::process_event(scap_evt *e, uint16_t devid) +int32_t libsinsp::sinsp_suppress::process_event(scap_evt *e) { if(m_suppressed_tids.empty() && m_suppressed_comms.empty()) { @@ -100,7 +100,7 @@ int32_t libsinsp::sinsp_suppress::process_event(scap_evt *e, uint16_t devid) comm = valptr; - if(is_suppressed_tid(*ptid, devid)) + if(is_suppressed_tid(*ptid)) { m_suppressed_tids.insert(e->tid); m_num_suppressed_events++; @@ -119,7 +119,6 @@ int32_t libsinsp::sinsp_suppress::process_event(scap_evt *e, uint16_t devid) auto it = m_suppressed_tids.find(e->tid); if (it != m_suppressed_tids.end()) { - cache_slot(devid) = 0; m_suppressed_tids.erase(it); m_num_suppressed_events++; return SCAP_FILTERED_EVENT; @@ -131,7 +130,7 @@ int32_t libsinsp::sinsp_suppress::process_event(scap_evt *e, uint16_t devid) } default: - if (is_suppressed_tid(e->tid, devid)) + if (is_suppressed_tid(e->tid)) { m_num_suppressed_events++; return SCAP_FILTERED_EVENT; @@ -143,25 +142,11 @@ int32_t libsinsp::sinsp_suppress::process_event(scap_evt *e, uint16_t devid) } } -bool libsinsp::sinsp_suppress::is_suppressed_tid(uint64_t tid, uint16_t devid) const +bool libsinsp::sinsp_suppress::is_suppressed_tid(uint64_t tid) const { if (tid == 0) { return false; } - if(devid != UINT16_MAX && cache_slot(devid) == tid) - { - return true; - } return m_suppressed_tids.find(tid) != m_suppressed_tids.end(); } - -uint64_t& libsinsp::sinsp_suppress::cache_slot(uint16_t devid) -{ - return m_cache[devid % CACHE_SIZE]; -} - -uint64_t libsinsp::sinsp_suppress::cache_slot(uint16_t devid) const -{ - return m_cache[devid % CACHE_SIZE]; -} diff --git a/userspace/libsinsp/sinsp_suppress.h b/userspace/libsinsp/sinsp_suppress.h index 6f1556add3..5f1526392b 100644 --- a/userspace/libsinsp/sinsp_suppress.h +++ b/userspace/libsinsp/sinsp_suppress.h @@ -38,25 +38,19 @@ class sinsp_suppress bool check_suppressed_comm(uint64_t tid, const std::string& comm); - int32_t process_event(scap_evt* e, uint16_t devid); + int32_t process_event(scap_evt* e); - bool is_suppressed_tid(uint64_t tid, uint16_t devid) const; + bool is_suppressed_tid(uint64_t tid) const; uint64_t get_num_suppressed_events() const { return m_num_suppressed_events; } uint64_t get_num_suppressed_tids() const { return m_suppressed_tids.size(); } protected: - inline uint64_t& cache_slot(uint16_t devid); - inline uint64_t cache_slot(uint16_t devid) const; - std::unordered_set m_suppressed_comms; std::unordered_set m_suppressed_tids; uint64_t m_num_suppressed_events = 0; - - static constexpr size_t CACHE_SIZE = 1024; - uint64_t m_cache[CACHE_SIZE] {}; }; } \ No newline at end of file