Overview - Herb

7 days30 days

Latest CI Pipeline Executions

Fuzzy

Succeeded
main
6ca2914f Engine: Compute relative file paths lazily (#2262) Compute `VisitorContext#relative_file_path` on demand instead of eagerly during context construction. `Herb::Engine` creates a context for every template, but the relative path is primarily consumed by diagnostics, overlays, debug output, and visitors that explicitly request it. Valid templates compiled without those features previously still paid for `Pathname#absolute?`, path joining, `Pathname#relative_path_from`, and `#to_s`. The public behavior is unchanged: `relative_file_path`, context hash access, merging, inspection, and serialization still return the same value. A regression test verifies that derivation does not happen during initialization. Split out of #1872. ## Benchmark Measured `Herb::Engine` compilation over [`marcoroth/herb-corpus`](https://github.com/marcoroth/herb-corpus) at `5560d823`, using Ruby 4.0.2. The benchmark compiled the 36,046 corpus templates accepted by both variants with a filename and project path, `escape: true`, no visitors, non-strict parsing, and Ruby validation disabled. Results are medians of three full-corpus runs: | metric | `main` (`cc3eb8bc`) | this branch | delta | |-------------------|:--------------------|:------------|:------------------------| | allocated objects | 88,907,732 | 79,115,256 | **-9,792,476 (-11.0%)** | | wall time | 9.21s | 7.94s | **-13.8%** |
by Joel Hawksley
Succeeded
main
0b819993 Engine: Use `String#match?` for boolean-context regexp guards in the compiler (#2235) ## What Switch three boolean-context regexp guards in `Engine::Compiler` from `=~` to `String#match?`: - `trailing_whitespace?` — `token[1] =~ /\s\z/` - `leading_whitespace?` — `token[1] =~ /\A\s/` - `at_line_start?` — `last_value =~ WHITESPACE_ONLY` and `last_value =~ TRAILING_INDENTATION` ## Why These match a token value against a regexp purely for a truthy/falsy result, but `=~` allocates a `MatchData` and assigns `$~` on every call. They run in hot paths while scanning and optimizing the token stream, so `String#match?` returns the same boolean without allocating. The `=~` sites that actually consume `Regexp.last_match` (e.g. `extract_leading_space`) are intentionally left as-is. ## Benchmark Compiling a large real-world `.erb` view corpus (~6,000 templates, from the github.com monolith) with `Herb::Engine` on Ruby 4.0.5: about **−0.7%** (−152,875) total objects allocated per full-corpus compile. Small but free. Full test suite passes (`2386 runs, 5462 assertions, 0 failures, 0 errors`).
by Joel Hawksley
Succeeded
main
1ce56409 Engine: single-pass optimize_tokens (#2232) Collapses the two-pass whitespace/text optimization in `Herb::Engine::Compiler#optimize_tokens` into a single pass over the raw token stream. Previously `optimize_tokens` did two passes: 1. `compact_whitespace_tokens` built a whole intermediate array (`tokens.map.with_index { ... }.compact`), dropping/relabeling whitespace tokens. 2. `optimize_tokens` re-scanned that array to merge consecutive text tokens, accumulating with `current_text += value`. This PR folds both into one pass: whitespace tokens are resolved against their neighbours in the *original* stream (dropped, or turned into text) and consecutive text is merged inline. `compact_whitespace_tokens` is removed. `optimize_tokens` runs once per template compile, so this is on the hot path for any large `ActionView`/ReActionView precompile. ### Implementation notes - No intermediate array: the `map.with_index + compact` allocation is gone; the surviving whitespace-vs-neighbour checks (`adjacent_whitespace?`, `whitespace_before_code_sequence?`) run against the original `tokens`/`index`, exactly as before. - Text is accumulated into a single buffer that is mutated with `current_text << value` instead of `current_text += value`, which reallocated and copied the whole accumulated string on every text token. - The buffer is seeded with `value.dup` so a (possibly frozen) token value is never mutated in place. ### Benchmark Measured end-to-end through `ViewPrecompiler.precompile` over the github/github monolith template corpus (**6,302 templates**), ReActionView ON (Herb path), Ruby 4.0.5. Both sides were run against the **same Herb `main` native extension** — only `lib/herb/engine/compiler.rb` differs between them — so the delta isolates this change. 2 boots × 5 timed iterations each. | Config | Time (median) | Time (min) | Allocated objects (median) | |---|---|---|---| | `main` baseline | 19.618s / 19.396s | 19.050s / 19.107s | ~32,044,000 | | this PR | 18.967s / 19.175s | 18.838s / 18.823s | ~30,988,000 | | **delta** | ~−1 to −3% | ~−1.2% | **−1,056,000 (−3.3%)** | The stable, reproducible win is **~1.06M fewer allocated objects per full precompile (−3.3%)**. Wall-clock is consistently favorable but smaller and noisier (~1–3%). Split out of #1872.
by Joel Hawksley
Succeeded
main
1ce56409 Engine: single-pass optimize_tokens (#2232) Collapses the two-pass whitespace/text optimization in `Herb::Engine::Compiler#optimize_tokens` into a single pass over the raw token stream. Previously `optimize_tokens` did two passes: 1. `compact_whitespace_tokens` built a whole intermediate array (`tokens.map.with_index { ... }.compact`), dropping/relabeling whitespace tokens. 2. `optimize_tokens` re-scanned that array to merge consecutive text tokens, accumulating with `current_text += value`. This PR folds both into one pass: whitespace tokens are resolved against their neighbours in the *original* stream (dropped, or turned into text) and consecutive text is merged inline. `compact_whitespace_tokens` is removed. `optimize_tokens` runs once per template compile, so this is on the hot path for any large `ActionView`/ReActionView precompile. ### Implementation notes - No intermediate array: the `map.with_index + compact` allocation is gone; the surviving whitespace-vs-neighbour checks (`adjacent_whitespace?`, `whitespace_before_code_sequence?`) run against the original `tokens`/`index`, exactly as before. - Text is accumulated into a single buffer that is mutated with `current_text << value` instead of `current_text += value`, which reallocated and copied the whole accumulated string on every text token. - The buffer is seeded with `value.dup` so a (possibly frozen) token value is never mutated in place. ### Benchmark Measured end-to-end through `ViewPrecompiler.precompile` over the github/github monolith template corpus (**6,302 templates**), ReActionView ON (Herb path), Ruby 4.0.5. Both sides were run against the **same Herb `main` native extension** — only `lib/herb/engine/compiler.rb` differs between them — so the delta isolates this change. 2 boots × 5 timed iterations each. | Config | Time (median) | Time (min) | Allocated objects (median) | |---|---|---|---| | `main` baseline | 19.618s / 19.396s | 19.050s / 19.107s | ~32,044,000 | | this PR | 18.967s / 19.175s | 18.838s / 18.823s | ~30,988,000 | | **delta** | ~−1 to −3% | ~−1.2% | **−1,056,000 (−3.3%)** | The stable, reproducible win is **~1.06M fewer allocated objects per full precompile (−3.3%)**. Wall-clock is consistently favorable but smaller and noisier (~1–3%). Split out of #1872.
by Joel Hawksley
Succeeded
main
4d2aa7ca Parser: Count errors in `error_count` so binding can skip the error walk (#2217) ## Summary `ParseResult#errors` collects errors by walking the **entire AST** on every call, allocating an intermediate array at every node, even when the template parsed with **no errors**, which is the overwhelmingly common case. Every `Herb::Engine` compile calls `parse_result.errors`, so this walk runs once per template. This PR makes the clean-template path free, and does it once for every binding: 1. **Count the errors in `libherb`, not in a binding.** `parser_options_T` already carried an `error_count` field for the `max_errors` cap. `herb_parse` now fills it with the exact total and hands it back to the caller, so Ruby, JavaScript, WASM, Java and Rust all get the count instead of Ruby alone. 2. **Skip the recursive walk when the count is zero.** Each binding exposes the count on its parse result and returns the top-level errors directly when it is zero. 3. **Collect recursive errors into a shared accumulator.** `recursive_errors` was `errors + compact_child_nodes.flat_map(&:recursive_errors)`, allocating throwaway arrays at every node. It now walks children iteratively into a single accumulator, in all four language bindings. Split out of #1872 so it can be reviewed and benchmarked on its own. ## Implementation notes The total comes from one allocation-free visitor walk at the end of `herb_parse` instead of counting as errors are constructed. Six sites attach errors directly instead of going through the generated `append_*` helper, including the Ruby parse errors in `analyze/parse_errors.c`, so counting at construction time reported zero errors for a template like `<% if condition without end %>` while the tree held one. The walk is exact regardless of how an error was attached. `herb_parse` only installs its own counter when the caller leaves `error_count` NULL. A binding that supplies one reads the total back, and a binding that does not gets `nil` and simply walks, so the fallback is safe by construction. `max_errors` now caps every error type instead of only the two tag-matching sites it reached before, which is what the option has always been documented to do. This is a behavior change. The corpus run reports 129 fewer diagnostics across 36,976 files with no file changing verdict. ## Benchmark Measured end-to-end through the `github/github` monolith's `ViewPrecompiler.precompile`, which compiles the full template set through the Herb engine (so `ParseResult#errors` is exercised once per template). Numbers are the full-precompile pass (`bin/rails runner`), comparing the vendored gem built from `main` vs. built with this change: | variant | wall clock | allocations | | --- | --- | --- | | baseline (`main`) | 34.254s | 63,842,961 | | this PR | 32.264s | 54,098,228 | **≈9.74M fewer allocations (−15.3%)** and **≈2.0s faster (−5.8%)** across the whole precompile. ## Correctness - Clean template → `error_count == 0`, the fast path returns the top-level errors and skips the walk. - Template with errors → the count matches the tree exactly, including Ruby parse errors and errors suppressed by `max_errors`. - New tests assert `error_count == recursive_errors.size` across every error path in Ruby, both JavaScript bindings, Java and Rust. - Two pre-existing bugs fixed along the way. The Node binding never read `max_errors`, so the cap silently did nothing there while WASM honoured it. Java seeded the parse result's error list from the document node, which the recursive walk then visited again, double counting every document-level error. - Ruby, C, Java, Rust and every JavaScript package suite passes. RuboCop and `cargo +nightly fmt --check` clean. --- *This change was produced by Claude Opus 4.8 (GitHub Copilot), acting on behalf of @joelhawksley.* --------- Co-authored-by: Marco Roth <marco.roth@intergga.ch>
by Joel Hawksley
Succeeded
main
e14fe9a7 Parser: Add `track_locations` option to skip source-location objects (#2199) Splits the `track_locations` parse option out of #1872 into its own PR, as requested. `Herb.parse` builds a `Location` (two `Position`s) — and a `Range` — for every AST node and token. Callers that never read source locations (e.g. rendering a template with validation disabled) pay to materialize objects they immediately discard; `Location`/`Range`/`Position` account for roughly half of the parse's Ruby allocations. This adds an additive `track_locations:` option to `Herb.parse` (default `true`, fully backward compatible). When `false`, the node/token builders leave `location` and `range` as `nil`. The flag is applied under the GVL immediately before Ruby AST materialization, so it cannot race with the native parse. It also makes `Herb::AST::Helpers#inline_ruby_comment?` safe when locations are not tracked. That helper detected a single-line inline `# comment` by comparing `node.location.start.line` to `node.location.end.line`, which raises once `track_locations: false` leaves the location `nil`. It runs on the compiler's happy path (`Compiler#visit_erb_content_node`), so without this fix any template containing an inline Ruby comment fails to compile when locations are off. It now falls back to an equivalent newline check on the node content, keeping compiled output byte-identical whether or not locations are tracked. ## Metrics Measured by running `ActionView::Precompiler` against GitHub.com's views directory (Ruby 4.0.5 + PRISM), comparing `track_locations` on (current default) vs. off: **End-to-end `ActionView::Precompiler` (6,302 templates):** | | compile time (median) | allocations | | --- | --- | --- | | locations tracked (default) | 21.53s | 39.3M | | `track_locations: false` | 20.91s | 32.5M | | **delta** | **~3% faster** | **~17% fewer** | The end-to-end wall-clock delta is modest because Herb parsing is only a fraction of total precompile work (render-call scanning, file I/O, and ActionView's `compile!` machinery dominate); the allocation reduction is the headline. **Isolated `Herb::Engine` compile of the same corpus (8,578 `.html.erb` files, validation disabled):** | | compile time (median) | allocations | | --- | --- | --- | | locations tracked (default) | 11.43s | 38.3M | | `track_locations: false` | 10.40s | 29.7M | | **delta** | **9.0% faster** | **22.5% fewer** | Compiled `Herb::Engine#src` output was verified byte-for-byte identical across all corpus templates with locations on vs. off (0 mismatches, 0 skips). ## Notes - The only public API change is the additive `track_locations:` option on `Herb.parse`. - Generated files (`ext/herb/nodes.c`, `ext/herb/error_helpers.c`) are not touched by this PR; the location builders they call already honor the flag. - Split out of #1872 per the request to land `track_locations` as its own PR. --------- Co-authored-by: Marco Roth <marco.roth@intergga.ch>
by Joel Hawksley