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.

Sign up for GitHub

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

refactor(userspace/libsinsp): polish and enable filter caching #1906

Merged
merged 2 commits into from
Jun 20, 2024
Merged

refactor(userspace/libsinsp): polish and enable filter caching #1906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf1dc71
refactor(userspace/libsinsp): polish and enable filter caching
jasondellaluce Jun 12, 2024
50ee9c9
test(userspace/libsinsp): cover filter caching
jasondellaluce Jun 12, 2024
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
17 changes: 0 additions & 17 deletions userspace/libsinsp/event.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,23 +41,6 @@ class sinsp_evt;
///////////////////////////////////////////////////////////////////////////////
// Event arguments
///////////////////////////////////////////////////////////////////////////////
enum filtercheck_field_flags
{
EPF_NONE = 0,
EPF_FILTER_ONLY = 1 << 0, ///< this field can only be used as a filter.
EPF_PRINT_ONLY = 1 << 1, ///< this field can only be printed.
EPF_ARG_REQUIRED = 1 << 2, ///< this field includes an argument, under the form 'property.argument'.
EPF_TABLE_ONLY = 1 << 3, ///< this field is designed to be used in a table and won't appear in the field listing.
EPF_INFO = 1 << 4, ///< this field contains summary information about the event.
EPF_CONVERSATION = 1 << 5, ///< this field can be used to identify conversations.
EPF_IS_LIST = 1 << 6, ///< this field is a list of values.
EPF_ARG_ALLOWED = 1 << 7, ///< this field optionally includes an argument.
EPF_ARG_INDEX = 1 << 8, ///< this field accepts numeric arguments.
EPF_ARG_KEY = 1 << 9, ///< this field accepts string arguments.
EPF_DEPRECATED = 1 << 10,///< this field is deprecated.
EPF_NO_TRANSFORMER = 1 << 11,///< this field cannot have a field transformer.
EPF_NO_RHS = 1 << 12,///< this field cannot have a right-hand side filter check, and cannot be used as a right-hand side filter check.
};

/** @defgroup event Event manipulation
* Classes to manipulate events, extract their content and convert them into strings.
Expand Down
108 changes: 88 additions & 20 deletions userspace/libsinsp/filter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,6 @@ bool sinsp_filter_expression::compare(sinsp_evt *evt)
return res;
}

bool sinsp_filter_expression::extract(sinsp_evt *evt, std::vector<extract_value_t>& values, bool sanitize_strings)
{
return false;
}

int32_t sinsp_filter_expression::get_expr_boolop() const
{
if(m_checks.size() <= 1)
Expand Down Expand Up @@ -149,9 +144,8 @@ int32_t sinsp_filter_expression::get_expr_boolop() const
///////////////////////////////////////////////////////////////////////////////
// sinsp_filter implementation
///////////////////////////////////////////////////////////////////////////////
sinsp_filter::sinsp_filter(sinsp *inspector)
sinsp_filter::sinsp_filter()
{
m_inspector = inspector;
m_filter = std::make_unique<sinsp_filter_expression>();
m_curexpr = m_filter.get();
}
Expand Down Expand Up @@ -193,25 +187,31 @@ void sinsp_filter::add_check(std::unique_ptr<sinsp_filter_check> chk)
///////////////////////////////////////////////////////////////////////////////
sinsp_filter_compiler::sinsp_filter_compiler(
sinsp* inspector,
const std::string& fltstr)
const std::string& fltstr,
std::shared_ptr<sinsp_filter_cache_factory> cache_factory)
: m_flt_str(fltstr),
m_factory(std::make_shared<sinsp_filter_factory>(inspector, m_default_filterlist))
m_factory(std::make_shared<sinsp_filter_factory>(inspector, m_default_filterlist)),
m_cache_factory(cache_factory)
{
}

sinsp_filter_compiler::sinsp_filter_compiler(
std::shared_ptr<sinsp_filter_factory> factory,
const std::string& fltstr)
const std::string& fltstr,
std::shared_ptr<sinsp_filter_cache_factory> cache_factory)
: m_flt_str(fltstr),
m_factory(factory)
m_factory(factory),
m_cache_factory(cache_factory)
{
}

sinsp_filter_compiler::sinsp_filter_compiler(
std::shared_ptr<sinsp_filter_factory> factory,
const libsinsp::filter::ast::expr* fltast)
const libsinsp::filter::ast::expr* fltast,
std::shared_ptr<sinsp_filter_cache_factory> cache_factory)
: m_flt_ast(fltast),
m_factory(factory)
m_factory(factory),
m_cache_factory(cache_factory)
{
}

Expand All @@ -235,9 +235,16 @@ std::unique_ptr<sinsp_filter> sinsp_filter_compiler::compile()
}
}

// make sure the cache factory is all set
if (!m_cache_factory)
{
// by default, use a factory that enables caching
m_cache_factory = std::make_shared<exprstr_sinsp_filter_cache_factory>();
}

// create new filter using factory,
// setup compiler state and start compilation
m_filter = m_factory->new_filter();
m_filter = std::make_unique<sinsp_filter>();
m_last_boolop = BO_NONE;
m_last_node_field = nullptr;
m_last_node_field_is_plugin = false;
Expand Down Expand Up @@ -325,10 +332,20 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::unary_check_expr*
{
throw sinsp_exception("filter error: missing field in left-hand of unary check");
}

auto check = std::move(m_last_node_field);
check->m_cmpop = str_to_cmpop(e->op);
check->m_boolop = m_last_boolop;
check_op_type_compatibility(*check);

// install cache in the check
sinsp_filter_cache_factory::node_info_t node_info;
node_info.m_field = check->get_transformed_field_info();
check->m_cache_metrics = m_cache_factory->new_metrics(e->left.get(), node_info);
check->m_extract_cache = m_cache_factory->new_extract_cache(e->left.get(), node_info);
node_info.m_compare_operator = check->m_cmpop;
check->m_compare_cache = m_cache_factory->new_compare_cache(e, node_info);

m_filter->add_check(std::move(check));
}

Expand Down Expand Up @@ -364,6 +381,25 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::binary_check_expr

auto left_from_plugin = m_last_node_field_is_plugin;
auto check = std::move(m_last_node_field);

// install cache on left-hand side extraction field
sinsp_filter_cache_factory::node_info_t node_info;
node_info.m_field = check->get_transformed_field_info();
check->m_cache_metrics = m_cache_factory->new_metrics(e->left.get(), node_info);
check->m_extract_cache = m_cache_factory->new_extract_cache(e->left.get(), node_info);

// if the extraction comes from a plugin-implemented ield, then
// we need to add a storage transformer as the cache may end up storing a
// shallow copy of the value pointers that are not valid anymore. Note that
// this should not change the right field's eligibility for caching, as
// the storage transformer does not alter the field's info.
auto left_has_storage = false;
if (left_from_plugin && check->m_extract_cache)
{
left_has_storage = true;
check->add_transformer(filter_transformer_type::FTR_STORAGE);
}

check->m_cmpop = str_to_cmpop(e->op);
check->m_boolop = m_last_boolop;
check_op_type_compatibility(*check);
Expand All @@ -390,12 +426,42 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::binary_check_expr
// * if yes, check if they are associated with the same plugin instance, otherwise, this is not an issue. We use the plugin name
// to understand if the plugin is the same.
// * if yes, add the `FTR_STORAGE` transformer to the lhs filter check.
//
// Note, adding a storage layer on only one of the two sides of the comparison is enough to solve the problem.
//
// However, we may have already added a storage modifier to the left field due to issues with caching,
// in which case we are good already.
auto right_from_plugin = m_last_node_field_is_plugin;
if (left_from_plugin && right_from_plugin)
if (!left_has_storage && left_from_plugin && right_from_plugin)
{
check->add_transformer(filter_transformer_type::FTR_STORAGE);
}

// install cache on right-hand side extraction field
auto prev_left_field_info = node_info.m_field;
node_info.m_field = m_last_node_field->get_transformed_field_info();
m_last_node_field->m_cache_metrics = m_cache_factory->new_metrics(e->right.get(), node_info);
// note: the `val(...)` transformer is a no-op and can be ignored for better extract cache reusage
const auto* cacheable_expr = e->right.get();
if (const auto* val_transf_expr = dynamic_cast<const libsinsp::filter::ast::field_transformer_expr*>(cacheable_expr);
val_transf_expr != nullptr && val_transf_expr->transformer == "val")
{
cacheable_expr = val_transf_expr->value.get();
}
m_last_node_field->m_extract_cache = m_cache_factory->new_extract_cache(cacheable_expr, node_info);

// similarly as above, if the right-hand side extraction comes from a
// plugin-implemented field, then we need to add an additional storage
// layer on it as well
if (right_from_plugin && m_last_node_field->m_extract_cache)
{
m_last_node_field->add_transformer(filter_transformer_type::FTR_STORAGE);
}

// restore node info and set rhs one for later cache installations
node_info.m_right_field = m_last_node_field->get_transformed_field_info();
node_info.m_field = prev_left_field_info;

// We found another field as right-hand side of the comparison
check->add_filter_value(std::move(m_last_node_field));
}
Expand All @@ -413,6 +479,13 @@ void sinsp_filter_compiler::visit(const libsinsp::filter::ast::binary_check_expr
add_filtercheck_value(check.get(), i, m_field_values[i]);
}
}

// install cache in the check comparison
// note: we don't need to re-install the metrics as the check is implemented
// by the same object responsible of the left-hand side field extraction
node_info.m_compare_operator = check->m_cmpop;
check->m_compare_cache = m_cache_factory->new_compare_cache(e, node_info);

m_filter->add_check(std::move(check));
}

Expand Down Expand Up @@ -534,11 +607,6 @@ sinsp_filter_factory::sinsp_filter_factory(sinsp *inspector,
{
}

std::unique_ptr<sinsp_filter> sinsp_filter_factory::new_filter() const
{
return std::make_unique<sinsp_filter>(m_inspector);
}

std::unique_ptr<sinsp_filter_check> sinsp_filter_factory::new_filtercheck(std::string_view fldname) const
{
return m_available_checks.new_filter_check_from_fldname(fldname,
Expand Down
17 changes: 8 additions & 9 deletions userspace/libsinsp/filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ class sinsp_filter_expression : public sinsp_filter_check
}

bool compare(sinsp_evt*) override;
bool extract(sinsp_evt*, std::vector<extract_value_t>& values, bool sanitize_strings = true) override;

void add_check(std::unique_ptr<sinsp_filter_check> chk);

Expand All @@ -81,7 +80,7 @@ class sinsp_filter_expression : public sinsp_filter_check
class SINSP_PUBLIC sinsp_filter
{
public:
sinsp_filter(sinsp* inspector);
sinsp_filter();
virtual ~sinsp_filter() = default;

bool run(sinsp_evt *evt);
Expand All @@ -94,8 +93,6 @@ class SINSP_PUBLIC sinsp_filter

private:
sinsp_filter_expression* m_curexpr;

sinsp* m_inspector;
};

class sinsp_filter_factory
Expand Down Expand Up @@ -163,8 +160,6 @@ class sinsp_filter_factory

virtual ~sinsp_filter_factory() = default;

virtual std::unique_ptr<sinsp_filter> new_filter() const;

virtual std::unique_ptr<sinsp_filter_check> new_filtercheck(std::string_view fldname) const;

virtual std::list<filter_fieldclass_info> get_fields() const;
Expand Down Expand Up @@ -206,7 +201,8 @@ class SINSP_PUBLIC sinsp_filter_compiler:
*/
sinsp_filter_compiler(
sinsp* inspector,
const std::string& fltstr);
const std::string& fltstr,
std::shared_ptr<sinsp_filter_cache_factory> cache_factory = nullptr);

/*!
\brief Constructs the compiler
Expand All @@ -217,7 +213,8 @@ class SINSP_PUBLIC sinsp_filter_compiler:
*/
sinsp_filter_compiler(
std::shared_ptr<sinsp_filter_factory> factory,
const std::string& fltstr);
const std::string& fltstr,
std::shared_ptr<sinsp_filter_cache_factory> cache_factory = nullptr);

/*!
\brief Constructs the compiler
Expand All @@ -229,7 +226,8 @@ class SINSP_PUBLIC sinsp_filter_compiler:
*/
sinsp_filter_compiler(
std::shared_ptr<sinsp_filter_factory> factory,
const libsinsp::filter::ast::expr* fltast);
const libsinsp::filter::ast::expr* fltast,
std::shared_ptr<sinsp_filter_cache_factory> cache_factory = nullptr);

/*!
\brief Builds a filtercheck tree and bundles it in sinsp_filter
Expand Down Expand Up @@ -272,6 +270,7 @@ class SINSP_PUBLIC sinsp_filter_compiler:
std::shared_ptr<libsinsp::filter::ast::expr> m_internal_flt_ast;
const libsinsp::filter::ast::expr* m_flt_ast = nullptr;
std::shared_ptr<sinsp_filter_factory> m_factory;
std::shared_ptr<sinsp_filter_cache_factory> m_cache_factory;
std::vector<message> m_warnings;
sinsp_filter_check_list m_default_filterlist;
};
Expand Down
Loading
Loading