Skip to content
New issue

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

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

Already on GitHub? # to your account

refactor(userspace/libsinsp): improve performance of endswith filter operator #1796

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

jasondellaluce
Copy link
Contributor

What type of PR is this?

/kind cleanup

Any specific area of the project related to this PR?

/area libsinsp

Does this PR require a change in the driver versions?

What this PR does / why we need it:

Doing some benchmarks, I noticed that the endswith operator is usually one of the least performing one we support because the current implementation may involve occasional allocation and string copies due to a const char* to const std::string& conversion. If used extensively in a filter or in Falco rules, that can have noticeable performance and memory impact.

Which issue(s) this PR fixes:

Special notes for your reviewer:

/milestone 0.16.0

I think this should be something to consider for the upcoming release, as quite minimal but useful. Will let the release managers decide.

Does this PR introduce a user-facing change?:

refactor(userspace/libsinsp): improve performance of endswith filter operator

…operator

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

Not a release manager, but I think this is super valuable!

/approve

and let the others decide re the second approve.

@poiana
Copy link
Contributor

poiana commented Apr 16, 2024

LGTM label has been added.

Git tree hash: 64c838d7637ac55d050df02198391034cb980937

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Apr 16, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, jasondellaluce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [FedeDP,incertum,jasondellaluce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@FedeDP
Copy link
Contributor

FedeDP commented Apr 16, 2024

Agree with 0.16.0! Let's go!

@poiana poiana merged commit 03e34ed into master Apr 16, 2024
40 checks passed
@poiana poiana deleted the fix/endswith-performance branch April 16, 2024 19:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants