Skip to content

Commit

Permalink
Revert "update(sinsp): implement suppressed tid cache in sinsp_suppress"
Browse files Browse the repository at this point in the history
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 6a9a0ec.

Fixes: #1724

Signed-off-by: Grzegorz Nosek <[email protected]>
  • Loading branch information
gnosek authored and poiana committed Mar 19, 2024
1 parent 2e38d22 commit 41f4564
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 29 deletions.
4 changes: 2 additions & 2 deletions userspace/libsinsp/sinsp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 4 additions & 19 deletions userspace/libsinsp/sinsp_suppress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
{
Expand Down Expand Up @@ -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++;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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];
}
10 changes: 2 additions & 8 deletions userspace/libsinsp/sinsp_suppress.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> m_suppressed_comms;
std::unordered_set<uint64_t> m_suppressed_tids;

uint64_t m_num_suppressed_events = 0;

static constexpr size_t CACHE_SIZE = 1024;
uint64_t m_cache[CACHE_SIZE] {};
};

}

0 comments on commit 41f4564

Please sign in to comment.