Skip to content

perf: populate route match context once across entries#44663

Open
rudrakhp wants to merge 1 commit intoenvoyproxy:mainfrom
rudrakhp:route_match_context
Open

perf: populate route match context once across entries#44663
rudrakhp wants to merge 1 commit intoenvoyproxy:mainfrom
rudrakhp:route_match_context

Conversation

@rudrakhp
Copy link
Copy Markdown
Member

Commit Message: perf: populate route match context once across entries
Additional Description: In case of multiple route entries these values are evaluated per route today which can be done only once to save compute.
Risk Level: Low
Testing: Existing tests + unit tests
Docs Changes: N/A
Release Notes: No
Platform Specific Features: N/A
[Optional Runtime guard:]
Fixes #20609

@rudrakhp rudrakhp force-pushed the route_match_context branch from f7f167e to 13f679c Compare April 26, 2026 18:08
@sschepens
Copy link
Copy Markdown
Contributor

sschepens commented Apr 27, 2026

doesn't this introduce aditional overhead in the case where none of these matches are being performed on routes? for example if no query param matching is used, we currently don't even parse it, we would now do so.

maybe it would be best to lazily initialize the fields of the context when needed? #20609 also suggested doing this lazily

@rudrakhp
Copy link
Copy Markdown
Member Author

You’re right. I initially went with that approach, but it introduced a has() check on every access, which I was trying to avoid. That said, since the code paths that actually require all these parameters are relatively rare, we might end up with higher overhead overall from a heuristic standpoint.

@rudrakhp rudrakhp force-pushed the route_match_context branch from 13f679c to 4b7b9ea Compare April 27, 2026 13:25
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
@rudrakhp rudrakhp force-pushed the route_match_context branch from 4b7b9ea to 0258ffb Compare April 27, 2026 13:28
Copy link
Copy Markdown
Contributor

@ravenblackx ravenblackx left a comment

Choose a reason for hiding this comment

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

For a change whose purpose is perf, it would be nice to include some benchmark tests, and the before/after results in the PR description.

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.

matcher helper for mutiple http route entry matching

4 participants