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

filters: optimize cel expression context with constant-time lookups #37057

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

agrawroh
Copy link
Contributor

@agrawroh agrawroh commented Nov 8, 2024

Description

This PR optimizes the CEL expression context implementation by replacing if-else chains with hash maps for O(1) lookups and improving memory management. This would enhance the performance for header and filter state access patterns while maintaining the same functionality.

This change introduces constant-time lookups using absl::flat_hash_map and improves the overall design through static initialization of lookup maps for better performance.


Commit Message: filters: optimize cel expression context with constant-time lookups
Additional Description: This commit replaces linear if-else chains with flat_hash_maps for property lookups in the CEL expression context. It also improves memory management and provides better encapsulation of implementation details.

Risk Level: Low

  • No functional changes
  • Only performance optimizations
  • Existing test coverage maintained

Testing:

  • All existing tests pass
  • No behavioral changes

Docs Changes: N/A - internal implementation change only
Release Notes: N/A

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #37057 was opened by agrawroh.

see: more, trace.

@agrawroh agrawroh changed the title attributes: refactor context expression implementation filters: optimize cel expression context with constant-time lookups Nov 8, 2024
@agrawroh agrawroh marked this pull request as ready for review November 8, 2024 10:55
@agrawroh agrawroh force-pushed the refactor_expr branch 5 times, most recently from 43b1f1a to f00f43b Compare November 8, 2024 21:19
@wbpcode
Copy link
Member

wbpcode commented Nov 9, 2024

Thanks. This is great. Any performance improvements are welcome if it does't break the security and our style.

But could you also add a benchmark test to prove it works as our expection? Thanks.

/wait

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants