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

Filterx initial string cache #471

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

Conversation

bazsi
Copy link
Member

@bazsi bazsi commented Jan 24, 2025

This PR implements the caching of trivial FilterXString instances (e.g. zero-length and single character numbers), which are commonly found in our log patterns as string values. Instead of allocating these all the time, use a cached instance.

This is "initial" string cache, as I have some pending patches to implement a larger table of strings that could be cached
similarly, but I got to make more testing with those to see how they impact performance.

bazsi added 3 commits January 24, 2025 15:34
The convention is that we are using FilterXObject pointers in interfaces,
and potentially check if the object at runtime matches our expectations.
By using FilterXString instances there, we typed constructors as well
as publish FilterXString as a struct. I am not against publishing
internal structs if they serve optimization purposes, but any value should
rather be passed around as objects and not specific types.

Signed-off-by: Balazs Scheidler <[email protected]>
@@ -146,7 +147,8 @@ filterx_getattr_new(FilterXExpr *operand, FilterXString *attr_name)
self->super.free_fn = _free;
self->operand = operand;

self->attr = (FilterXObject *) attr_name;
filterx_object_is_type(attr_name, &FILTERX_TYPE_NAME(string));
self->attr = attr_name;
Copy link
Member

@MrAnno MrAnno Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we reverting this?
This was needed for #252 to guarantee type safety at compile time. In general, I consider it a good idea to achieve type safety with the C compiler wherever it is possible. For #252, it was possible and also needed.

The filterx_object_is_type() call here is a NOP.

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.

2 participants