-
-
Notifications
You must be signed in to change notification settings - Fork 724
perf(parser): remove bounds check for getting strings from lexer #16135
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
base: main
Are you sure you want to change the base?
perf(parser): remove bounds check for getting strings from lexer #16135
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
53034be to
9d60b34
Compare
b3e1014 to
891e0b4
Compare
9d60b34 to
5c73320
Compare
CodSpeed Performance ReportMerging #16135 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR optimizes the get_string method in the lexer by replacing safe string slicing operations with unsafe get_unchecked to eliminate bounds checking. The optimization targets a hot path that profiling showed took 1% of parsing time on binder.ts, achieving a +1% performance improvement across all parser benchmarks.
Key Changes:
- Refactored string extraction to compute adjusted start/end positions before slicing
- Replaced safe slice operations with
unsafe { get_unchecked(start..end) } - Added comprehensive debug assertions to validate safety invariants
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5c73320 to
099a5ce
Compare
099a5ce to
811c1a4
Compare
811c1a4 to
1fca5af
Compare
1fca5af to
45740bd
Compare

One profile of the parser showed that this function took up 1% of the time on
binder.ts. It seemed like this is a case where we could possibly remove the bounds checks, since our tokens are guaranteed to be contained within the source string. Since this is called really often, it's worth optimizing.+1% on all parser benchmarks: