Implement path lookahead and optimize normalization#188
Open
NotAShelf wants to merge 3 commits into
Open
Conversation
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: Id54f99e66b308f5464138b55ae599fb96a6a6964
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I71d00d96426a07117a63fc2a764dfe336a6a6964
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: Ifa62fc405065b078a93f3fe8445479d16a6a6964
NotAShelf
marked this pull request as ready for review
July 19, 2026 11:11
Member
Author
|
Okay, should be ready. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've gotten really into parser development recently (for the purposes of witing a nftables parser) and ultimately ended up needing/wanting/dreaming of one for Nix. While looking for inspiration in rnix's codebase I've identified some areas that I could be "optimized". If my math is correct, they return rather positive results:
tokenizer/all-packagesnormalized-parts/all-packagesMy motivation for the change is rather simple. The way I see it, our path/URI disambiguation code scans an initial run of path characters before it knows what kind of token it is looking at. That scan used to be materialized as a
String, despite the following decision only needing two things from it:Recording those while scanning keeps the existing classification rules without allocating or walking the prefix again. Thus, this PR removes the temporary
Stringallocation, and lookahead now records the byte offset and whether it saw an underscore while scanning the input. On a similar optimization now (which is minor in comparisonStr::normalized_partsno longer collects all string parts before processing them but instead, it makes separate passes over the immutable syntax tree for indentation detection and output generation, and preallocates the result buffers for us. On rnix's own fixtures, tokenizer time dropped from about 25.5 ms to 6.0 ms, and string normalization dropped from about 301 µs to 87 µs. Which I see as an absolute win even though the numbers are rather small.I've went back in the git history to see if this changes are intentional by any chance and looks like the temporary
Stringdates to all the way back tothe original 2019 URI heuristic. Admittedly it was a straightforward way to retain both "how far did I scan?" and "did I see_?" but I think we can do better.The public API remains the same; the optimization is internal, but I do want to state that
unescapenow reservesinput.len()even when an escape-heavy string will shrink substantially. This can, in theory, reserve more transient memory for a huge, escape-dense string than the old growth strategy. Should probably not matter for most cases.I've written a tiny benchmark suite to get the numbers. I'm keeping this PR as a draft while I beat it into shape, but in the meantime please do feel free to tear apart my changes. I've written this most of those changes while rather exhausted and mostly with the goal of getting them off my mind before I call it a day. If I misread anything or misunderstood any intent anywhere, please let me know. I'm happy to iterate.
P.S. I'm sorry for the wall of text. I do hope this doesn't defer from reviewing the actual change, as it's very simple in theory and in application. I like typing a bit too much.