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

WIP: the tip of my filterx performance efforts #434

Open
wants to merge 44 commits into
base: main
Choose a base branch
from

Conversation

bazsi
Copy link
Member

@bazsi bazsi commented Dec 30, 2024

Please ignore this PR as it is not intended to be merged. It contains work-in-progress local patches I am yet to extract into separate pull requests.

It is rebased on top of my local merge of all related PRs, so I can maintain those branches separately as they get merged.

bazsi added 19 commits December 28, 2024 07:41
Instead of coding this individually in all FilterXExpr derivatives.

Signed-off-by: Balazs Scheidler <[email protected]>
This new method can be used to rewrite the expression tree after parsing
in order to execute faster. This is where we can check literal values
and potentially evaluate literal expressions.

Signed-off-by: Balazs Scheidler <[email protected]>
We incorrectly called filterx_expr_init_method() directly instead of
going through the virtual function pointer.

Signed-off-by: Balazs Scheidler <[email protected]>
Ubuntu has -fno-omit-frame-pointer in CFLAGS which Debian testing does
not have, making profiling a lot more difficult on Debian.

Signed-off-by: Balazs Scheidler <[email protected]>
Only exprs added as a stmt_expr had location information, meaning
that conditions or exprs in ternaries did not have location information.

Add those as well.

Signed-off-by: Balazs Scheidler <[email protected]>
Sometimes (especially with location tracking bugs) a grammar location
may be outside of the dimensions of the source text. Make sure we
never index outside of the dimensions of the "lines" array.

Signed-off-by: Balazs Scheidler <[email protected]>
… is enabled

This is a performance optimization that should eliminate filter_object_repr()
calls from the profile.

Signed-off-by: Balazs Scheidler <[email protected]>
The special handling for $MSG is long gone, avoid handling $MSG as a macro
as it requires extra copying in filterx and there's no functional
requirement anymore.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the filterx-perf-project branch from 3d96664 to c12a771 Compare December 30, 2024 13:16
bazsi added 10 commits December 30, 2024 15:53
…mentation

Instead of passing the arguments in a GPtrArray, let's use a simple stack
allocated array, voiding a 2 mallocs and 2 frees in any simple function
invocations.

Signed-off-by: Balazs Scheidler <[email protected]>
Optimize the removal of multiple elements.

Signed-off-by: Balazs Scheidler <[email protected]>
Instead of doing this in the hot path. Also eliminate the related helper
function.

Signed-off-by: Balazs Scheidler <[email protected]>
… non-zero

To match how object-bytes/object-protobuf works.

Signed-off-by: Balazs Scheidler <[email protected]>
It took me a while to realize why the output is not rendered in our
light tests, once I removed $MSG as a macro. This message would have
helped me a lot.

Signed-off-by: Balazs Scheidler <[email protected]>
With the $MSG as a macro gone, $MSG became a typed value, so it does matter
what we set it to. Since setting it to a bytes/protobuf value means
that its not a string anymore, we can't render it as a part of a template
unless we explicitly ask for the right type hint.

This is what this patch changes in the broken testcases.

Signed-off-by: Balazs Scheidler <[email protected]>
# Conflicts:
#	lib/filterx/filterx-grammar.ym
# Conflicts:
#	lib/filterx/expr-generator.c
#	lib/filterx/filterx-expr.c
#	lib/filterx/filterx-expr.h
bazsi added 15 commits December 31, 2024 12:06
To make it easier to understand stackdumps.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the filterx-perf-project branch from c12a771 to 64e6eb5 Compare December 31, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant