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

getFiltersForPolicy is a performance hotspot #123

Closed
RobDickinson opened this issue Aug 9, 2024 · 2 comments
Closed

getFiltersForPolicy is a performance hotspot #123

RobDickinson opened this issue Aug 9, 2024 · 2 comments

Comments

@RobDickinson
Copy link
Collaborator

phileas-benchmark profiling shows that significant CPU time can be spent in the getFiltersForPolicy method (even with the recent addition of caching there). It might be worth considering a second level of caching where the result lists themselves don't have to be created each time?

Screenshot 2024-07-30 at 1 21 36 AM

@RobDickinson
Copy link
Collaborator Author

@jzonthemtn having revisted this with latest code I think this is just an artifact of how this benchmark works.

If you process very small strings (<32 bytes), hundreds of thousands of times per second, and do nothing else, then the overhead for getFiltersForPolicy can look significant like I showed originally. (and that's what I was doing at the time, microbenchmarking with mostly small strings)

Now that we have some real and non-microbenchmarky integrations, I did some analysis on a few more realistic workloads, and I didn't see one where getFiltersForPolicy was significant.

I think I was up pretty late and mis-interpreted this as an opportunity to optimize, cause who wouldn't want ~20%? 😀

I humbly withdraw my complaint

@jzonthemtn
Copy link
Member

Sounds good! Appreciate the update.

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

No branches or pull requests

2 participants