Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup(userspace/libsinsp): reduce allocations in filtercheck classes #1680

Merged
merged 4 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions userspace/libsinsp/filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ bool sinsp_filter_expression::compare(sinsp_evt *evt)
bool res = true;

sinsp_filter_check* chk = nullptr;
++m_hits;

auto size = m_checks.size();
for(size_t j = 0; j < size; j++)
Expand Down Expand Up @@ -126,10 +125,6 @@ bool sinsp_filter_expression::compare(sinsp_evt *evt)
}
}
done:
if (res)
{
m_matched_true++;
}
return res;
}

Expand Down Expand Up @@ -266,16 +261,9 @@ sinsp_filter* sinsp_filter_compiler::compile()

// create new filter using factory
auto new_filter = m_factory->new_filter();
auto new_sinsp_filter = dynamic_cast<sinsp_filter*>(new_filter);
if (new_sinsp_filter == nullptr)
{
ASSERT(false);
delete new_filter;
throw sinsp_exception("filter error: factory did not create a sinsp_filter");
}

// setup compiler state and start compilation
m_filter = new_sinsp_filter;
m_filter = new_filter;
m_last_boolop = BO_NONE;
m_expect_values = false;
try
Expand All @@ -284,14 +272,14 @@ sinsp_filter* sinsp_filter_compiler::compile()
}
catch (const sinsp_exception& e)
{
delete new_sinsp_filter;
delete new_filter;
m_filter = NULL;
throw e;
}

// return compiled filter
m_filter = NULL;
return new_sinsp_filter;
return new_filter;
}

void sinsp_filter_compiler::visit(const libsinsp::filter::ast::and_expr* e)
Expand Down
115 changes: 59 additions & 56 deletions userspace/libsinsp/sinsp_filtercheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ limitations under the License.
#include <libsinsp/sinsp_filtercheck.h>
#include <libsinsp/value_parser.h>

#define STRPROPERTY_STORAGE_SIZE 1024

#ifndef _GNU_SOURCE
//
// Fallback implementation of memmem
Expand Down Expand Up @@ -861,10 +863,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(int8_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_INT16:
if(print_format == PF_OCT)
{
Expand All @@ -885,10 +888,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(int16_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_INT32:
if(print_format == PF_OCT)
{
Expand All @@ -909,10 +913,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(int32_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_INT64:
case PT_PID:
case PT_ERRNO:
Expand All @@ -939,10 +944,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
prfmt = (char*)"%" PRId64;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(int64_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_L4PROTO: // This can be resolved in the future
case PT_UINT8:
if(print_format == PF_OCT)
Expand All @@ -964,10 +970,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(uint8_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_PORT: // This can be resolved in the future
case PT_UINT16:
if(print_format == PF_OCT)
Expand All @@ -989,10 +996,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(uint16_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_UINT32:
if(print_format == PF_OCT)
{
Expand All @@ -1013,10 +1021,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(uint32_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_UINT64:
case PT_RELTIME:
case PT_ABSTIME:
Expand All @@ -1042,11 +1051,12 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
ASSERT(false);
return NULL;
}

snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),

m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
prfmt, *(uint64_t *)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_CHARBUF:
case PT_FSPATH:
case PT_FSRELPATH:
Expand All @@ -1060,10 +1070,11 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
}
else
{
auto copy_len = std::min(len, (uint32_t) sizeof(m_getpropertystr_storage));
memcpy(m_getpropertystr_storage, rawval, copy_len);
m_getpropertystr_storage[copy_len] = 0;
return m_getpropertystr_storage;
auto copy_len = std::min(len, (uint32_t) STRPROPERTY_STORAGE_SIZE);
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
memcpy(m_getpropertystr_storage.data(), rawval, copy_len);
m_getpropertystr_storage.data()[copy_len] = 0;
return m_getpropertystr_storage.data();
}
case PT_SOCKADDR:
ASSERT(false);
Expand All @@ -1081,14 +1092,15 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return (char*)"false";
}
case PT_IPV4ADDR:
snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
"%" PRIu8 ".%" PRIu8 ".%" PRIu8 ".%" PRIu8,
rawval[0],
rawval[1],
rawval[2],
rawval[3]);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_IPV6ADDR:
{
char address[INET6_ADDRSTRLEN];
Expand All @@ -1098,9 +1110,10 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
strlcpy(address, "<NA>", INET6_ADDRSTRLEN);
}

strlcpy(m_getpropertystr_storage, address, sizeof(m_getpropertystr_storage));
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
strlcpy(m_getpropertystr_storage.data(), address, STRPROPERTY_STORAGE_SIZE);

return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
}
case PT_IPADDR:
if(len == sizeof(struct in_addr))
Expand All @@ -1117,15 +1130,17 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
}

case PT_DOUBLE:
snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
"%.1lf", *(double*)rawval);
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
case PT_IPNET:
snprintf(m_getpropertystr_storage,
sizeof(m_getpropertystr_storage),
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
snprintf(m_getpropertystr_storage.data(),
STRPROPERTY_STORAGE_SIZE,
"<IPNET>");
return m_getpropertystr_storage;
return m_getpropertystr_storage.data();
default:
ASSERT(false);
throw sinsp_exception("wrong param type " + std::to_string((long long) ptype));
Expand All @@ -1152,8 +1167,9 @@ char* sinsp_filter_check::tostring(sinsp_evt* evt)
res += rawval_to_string(val.ptr, m_field->m_type, m_field->m_print_format, val.len);
}
res += ")";
strlcpy(m_getpropertystr_storage, res.c_str(), sizeof(m_getpropertystr_storage));
return m_getpropertystr_storage;
m_getpropertystr_storage.resize(STRPROPERTY_STORAGE_SIZE);
strlcpy(m_getpropertystr_storage.data(), res.c_str(), STRPROPERTY_STORAGE_SIZE);
return m_getpropertystr_storage.data();
}
return rawval_to_string(m_extracted_values[0].ptr, m_field->m_type, m_field->m_print_format, m_extracted_values[0].len);
}
Expand Down Expand Up @@ -1550,7 +1566,6 @@ bool sinsp_filter_check::extract(sinsp_evt *evt, OUT std::vector<extract_value_t

bool sinsp_filter_check::compare(sinsp_evt* evt)
{
m_hits++;
if(m_cache_metrics != NULL)
{
m_cache_metrics->m_num_eval++;
Expand All @@ -1575,23 +1590,11 @@ bool sinsp_filter_check::compare(sinsp_evt* evt)
}
}

if (m_eval_cache_entry->m_res)
{
m_matched_true++;
}
m_cached++;

return m_eval_cache_entry->m_res;
}
else
{
auto res = compare_nocache(evt);
if (res)
{
m_matched_true++;
}

return res;
return compare_nocache(evt);
}
}

Expand Down
6 changes: 1 addition & 5 deletions userspace/libsinsp/sinsp_filtercheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,6 @@ class sinsp_filter_check
check_cache_metrics *m_cache_metrics = nullptr;
boolop m_boolop = BO_NONE;
cmpop m_cmpop = CO_NONE;
size_t m_hits = 0;
size_t m_cached = 0;
size_t m_matched_true = 0;

char* rawval_to_string(uint8_t* rawval,
ppm_param_type ptype,
Expand Down Expand Up @@ -267,12 +264,11 @@ class sinsp_filter_check
bool compare_rhs(cmpop op, ppm_param_type type, std::vector<extract_value_t>& values);

Json::Value rawval_to_json(uint8_t* rawval, ppm_param_type ptype, ppm_print_format print_format, uint32_t len);
void string_to_rawval(const char* str, uint32_t len, ppm_param_type ptype);

inline uint8_t* filter_value_p(uint16_t i = 0) { return &m_val_storages[i][0]; }
inline std::vector<uint8_t>* filter_value(uint16_t i = 0) { return &m_val_storages[i]; }

char m_getpropertystr_storage[1024];
std::vector<char> m_getpropertystr_storage;
std::vector<std::vector<uint8_t>> m_val_storages;

std::vector<filter_value_t> m_vals;
Expand Down
7 changes: 4 additions & 3 deletions userspace/libsinsp/sinsp_filtercheck_container.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ limitations under the License.
#include <libsinsp/sinsp.h>
#include <libsinsp/sinsp_filtercheck.h>

class sinsp_filter_check_reference;

class sinsp_filter_check_container : public sinsp_filter_check
{
public:
Expand Down Expand Up @@ -55,13 +53,16 @@ class sinsp_filter_check_container : public sinsp_filter_check
};

sinsp_filter_check_container();
virtual ~sinsp_filter_check_container() = default;

sinsp_filter_check* allocate_new() override;
int32_t parse_field_name(const char* str, bool alloc_state, bool needed_for_filtering) override;
uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override;

const std::string& get_argstr() const;

protected:
uint8_t* extract(sinsp_evt*, OUT uint32_t* len, bool sanitize_strings = true) override;

private:
int32_t extract_arg(const std::string& val, size_t basename);

Expand Down
14 changes: 0 additions & 14 deletions userspace/libsinsp/sinsp_filtercheck_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,10 @@ sinsp_filter_check_event::sinsp_filter_check_event()
m_info.m_nfields = sizeof(sinsp_filter_check_event_fields) / sizeof(sinsp_filter_check_event_fields[0]);
m_u64val = 0;
m_converter = new sinsp_filter_check_reference();

m_storage_size = UESTORAGE_INITIAL_BUFSIZE;
m_storage = (char*)malloc(m_storage_size);
if(m_storage == NULL)
{
throw sinsp_exception("memory allocation error in sinsp_filter_check_appevt::sinsp_filter_check_event");
}

m_cargname = NULL;
}

sinsp_filter_check_event::~sinsp_filter_check_event()
{
if(m_storage != NULL)
{
free(m_storage);
}

if(m_converter != NULL)
{
delete m_converter;
Expand Down
Loading
Loading