Engine: Skip location tracking when nothing asks for it - #2317
Merged
Merged
Conversation
This pull request stops the engine from building a `Location`, two `Position`s and a `Range` for every node and token when nothing is going to read them. `Herb.parse` attaches source locations to the whole tree. Profiling `Herb::Engine.new` over the `examples/` corpus showed that those objects are the single largest allocation source in a compile, well ahead of anything in the compiler itself: | class | objects | share of a compile | | --- | ---: | ---: | | `Herb::Position` | 27,124 | 23.2% | | `Herb::Location` | 13,562 | 11.6% | | `Herb::Range` | 7,308 | 6.3% | | **total** | **47,994** | **41.1%** | Nothing in the default compile path reads any of them. `Compiler` never touches `node.location`, and `report` returns early when no visitor reports diagnostics. The consumers are all visitors, which the caller supplies. So when the visitor stack is empty, the engine now parses with `track_locations: false`, the option #2199 added for exactly this. A caller that sets `track_locations` itself still gets what it asked for, in either direction. #### Errors keep their locations A parse error carries its own location, and `track_locations` does not touch it. Error messages are byte-identical with tracking off, carets and all, so the error path needs no second parse. #### Results Measured A/B interleaved against `main` over the `examples/` corpus: | metric | `main` | this branch | delta | | --- | ---: | ---: | ---: | | allocated objects | 116,723 | 68,795 | **-47,928 (-41.1%)** | | wall time | 11.43 ms | 9.00 ms | **-21%** | Compiled output is byte-identical across all 128 `.erb` templates in the repository, each compiled in three configurations.
`with_element_context` downcases the tag name it is given, and `visit_html_element_node` then downcased the same tag name again inside the block that context yields to. Every element paid for two `String`s where one would do, since `downcase` allocates even when the name is already lowercase. The context now yields the name it already has. Exiting an element also built a fresh `["script", "style"]` on every call to decide whether to pop a raw-text context. That array is now a frozen constant. Together this is 1,910 fewer objects over the `examples/` corpus, about 2.8% of what a compile allocates after #2199's location option is applied. Compiled output is byte-identical across all 128 `.erb` templates in the repository.
github-actions
Bot
added
ruby
marcoroth
added
optimization
|
View your CI Pipeline Execution ↗ for commit d6aaa74
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
The rendered error message draws its gutter with a box character or an ASCII pipe depending on the environment, so matching the message text failed outside the environment it was written in. Assert on the diagnostic's location instead.
`OptimizeVisitor` collects a location per element so the guard it compiles in can say where an overwritten helper was used, but it only compiles that guard when `verify` is on, and `verify` defaults to off. With the class declaring the option, every Action View compile paid for a full set of location objects to fill a hash nothing read. The declaration mechanism resolves options from the visitor instance, so the requirement can depend on how the visitor was built. `OptimizeVisitor` now asks for `track_locations` only when it is going to verify. Over the 35,875 corpus templates that compile with the visitor attached, with `verify` left at its default: | metric | before | after | delta | | --- | ---: | ---: | ---: | | allocated objects | 91,861,776 | 57,585,374 | -34,276,402 (-37.3%) | | wall time | 13.56s | 11.44s | -15.6% | Compiled output is byte-identical across those templates.
marcoroth
enabled auto-merge (squash)
August 20, 2026 06:34
marcoroth
changed the title
marcoroth
disabled auto-merge
August 20, 2026 06:34
marcoroth
enabled auto-merge (squash)
August 20, 2026 06:35
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
1 participant
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.
This pull request stops the engine from building a
Location, twoPositions and aRangefor every node and token when nothing is going to read them.Herb.parseattaches source locations to the whole tree. ProfilingHerb::Engine.newwithObjectSpaceallocation tracing showed those objects are the single largest allocation source in a compile, well ahead of anything in the compiler itself:Herb::PositionHerb::LocationHerb::RangeLocations are now off unless something asks for them.
Compilernever readsnode.location, so a template compiled without a visitor never needs one.Letting a visitor say what it needs
Every built-in visitor that reads
node.locationdeclaresrequired_parser_option track_locations: true, andHerb::Visitor.parser_options_foralready folds those declarations into the options the engine parses with, so the engine needs no list of its own.Herb::Engine::Diagnosticsdeclares it on include. Any visitor that reports a diagnostic keeps its locations without knowing this option exists, including visitors from outside this repository, since reporting is the usual reason to want a location at all. The engine's ownDiagnosticsTestcaught this before the declaration was added.A visitor that reads locations, reports nothing, and declares nothing gets
nil. That is the one behaviour this trades away.Building on #2199
#2199 added
track_locationsfor callers that never read source locations, naming "rendering a template with validation disabled" as the case it had in mind. This turns that case on automatically, since the engine can tell when it applies without the caller having to know.That pull request also did the work that makes this safe.
Herb::AST::Helpers#inline_ruby_comment?used to comparenode.location.start.lineagainstnode.location.end.line, and it runs on the compiler's happy path throughCompiler#visit_erb_content_node, so it raised as soon as a location was nil. #2199 replaced that with an equivalent newline check on the node content. Without it, every template containing an inline<% # comment %>would fail to compile here.Errors keep their locations
A parse error carries its own location and
track_locationsdoes not touch it, so the error path needs no second parse. Error messages are byte-identical with tracking off, carets and all.Results
Measured over
marcoroth/herb-corpusat12cced87, the 35,881 templates of 36,989 that compile cleanly, Ruby 4.0.2. Samples interleaved withmain:mainCompiled output is byte-identical across the corpus: one SHA256 over all 35,881 compiled sources matches
mainexactly (3b3f9279…).