From 70a40ee5f637303dfb604a18642bb67d675ccaa7 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 24 Jul 2026 14:59:29 -0600 Subject: [PATCH 1/6] Add track_locations parse option to skip source-location objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Herb builds a Location (two Positions) for every AST node and token during parsing. Callers that never read source locations — e.g. rendering a template with validation disabled — pay for materializing objects they immediately discard. Location/Range/Position account for roughly half of the parse's Ruby allocations and a meaningful share of its time. Add a `track_locations:` option to `Herb.parse` (default true, fully backward compatible). When false, the node/token builders leave location and range as nil. The AST value objects are also built via direct allocation + ivar set, which is what lets the location/range builders bail out before allocating. The flag is applied under the GVL immediately before Ruby AST materialization, so it cannot race with the native parse. --- ext/herb/extension.c | 20 +++++++++ ext/herb/extension_helpers.c | 82 ++++++++++++++++++++++++++++-------- ext/herb/extension_helpers.h | 7 +++ 3 files changed, 91 insertions(+), 18 deletions(-) diff --git a/ext/herb/extension.c b/ext/herb/extension.c index 0d59d68a6..b1bc23417 100644 --- a/ext/herb/extension.c +++ b/ext/herb/extension.c @@ -23,6 +23,7 @@ typedef struct { VALUE source; const parser_options_T* parser_options; hb_allocator_T allocator; + bool track_locations; } parse_args_T; typedef struct { @@ -39,6 +40,11 @@ typedef struct { static VALUE parse_convert_body(VALUE arg) { parse_args_T* args = (parse_args_T*) arg; + // Apply here (rather than before the native parse) so the flag is set under + // the GVL immediately before Ruby AST materialization, independent of whether + // the native parse released the GVL. + herb_ext_track_locations = args->track_locations; + return create_parse_result(args->root, args->source, args->parser_options); } @@ -194,9 +200,23 @@ static VALUE Herb_parse(int argc, VALUE* argv, VALUE self) { if (!NIL_P(arena_stats) && RTEST(arena_stats)) { print_arena_stats = true; } } + // Whether to materialize Location/Range/Position objects on the Ruby AST. + // Defaults to true. Callers that never read source locations (e.g. rendering + // with validation disabled) can pass `track_locations: false` to skip ~half + // of the parse's Ruby allocations. Tracked outside parser_options_T since it + // only affects Ruby materialization, not the native parse, and applied in + // parse_convert_body just before AST materialization. + bool track_locations = true; + if (!NIL_P(options)) { + VALUE track_locations_opt = rb_hash_lookup(options, rb_utf8_str_new_cstr("track_locations")); + if (NIL_P(track_locations_opt)) { track_locations_opt = rb_hash_lookup(options, ID2SYM(rb_intern("track_locations"))); } + if (!NIL_P(track_locations_opt) && !RTEST(track_locations_opt)) { track_locations = false; } + } + parse_args_T args = { 0 }; args.source = source; args.parser_options = &parser_options; + args.track_locations = track_locations; if (!hb_allocator_init(&args.allocator, HB_ALLOCATOR_ARENA)) { return Qnil; } diff --git a/ext/herb/extension_helpers.c b/ext/herb/extension_helpers.c index eb0663020..0387fce25 100644 --- a/ext/herb/extension_helpers.c +++ b/ext/herb/extension_helpers.c @@ -1,5 +1,7 @@ #include +#include + #include "extension.h" #include "extension_helpers.h" #include "nodes.h" @@ -21,28 +23,71 @@ const char* check_string(VALUE value) { return RSTRING_PTR(value); } +// Whether to materialize source Location/Range/Position objects while building +// the Ruby AST. For rendering (no validation, no debug), locations are never +// read, yet they account for ~50% of parse allocations and ~28% of parse time +// (each node and token otherwise builds a Location from two Positions). When a +// caller passes `track_locations: false` to Herb.parse, these are left as nil. +// Set per-parse from Herb_parse; safe because AST materialization runs under +// the GVL, so no two parses materialize concurrently. +bool herb_ext_track_locations = true; + +// Cached instance-variable IDs for the hot AST value objects (Position, +// Location, Range, Token). Building these via rb_obj_alloc + rb_ivar_set sets +// the ivars directly, mirroring the Ruby classes' initializers. +static ID id_line, id_column, id_start, id_end, id_from, id_to; +static ID id_value, id_range, id_location, id_type; +static bool ast_value_ivar_ids_initialized = false; + +static void init_ast_value_ivar_ids(void) { + if (ast_value_ivar_ids_initialized) { return; } + + id_line = rb_intern("@line"); + id_column = rb_intern("@column"); + id_start = rb_intern("@start"); + id_end = rb_intern("@end"); + id_from = rb_intern("@from"); + id_to = rb_intern("@to"); + id_value = rb_intern("@value"); + id_range = rb_intern("@range"); + id_location = rb_intern("@location"); + id_type = rb_intern("@type"); + + ast_value_ivar_ids_initialized = true; +} + VALUE rb_position_from_c_struct(position_T position) { - VALUE args[2]; - args[0] = UINT2NUM(position.line); - args[1] = UINT2NUM(position.column); + init_ast_value_ivar_ids(); + + VALUE obj = rb_obj_alloc(cPosition); + rb_ivar_set(obj, id_line, UINT2NUM(position.line)); + rb_ivar_set(obj, id_column, UINT2NUM(position.column)); - return rb_class_new_instance(2, args, cPosition); + return obj; } VALUE rb_location_from_c_struct(location_T location) { - VALUE args[2]; - args[0] = rb_position_from_c_struct(location.start); - args[1] = rb_position_from_c_struct(location.end); + if (!herb_ext_track_locations) { return Qnil; } - return rb_class_new_instance(2, args, cLocation); + init_ast_value_ivar_ids(); + + VALUE obj = rb_obj_alloc(cLocation); + rb_ivar_set(obj, id_start, rb_position_from_c_struct(location.start)); + rb_ivar_set(obj, id_end, rb_position_from_c_struct(location.end)); + + return obj; } VALUE rb_range_from_c_struct(range_T range) { - VALUE args[2]; - args[0] = UINT2NUM(range.from); - args[1] = UINT2NUM(range.to); + if (!herb_ext_track_locations) { return Qnil; } + + init_ast_value_ivar_ids(); + + VALUE obj = rb_obj_alloc(cRange); + rb_ivar_set(obj, id_from, UINT2NUM(range.from)); + rb_ivar_set(obj, id_to, UINT2NUM(range.to)); - return rb_class_new_instance(2, args, cRange); + return obj; } VALUE rb_string_from_hb_string(hb_string_T string) { @@ -54,14 +99,15 @@ VALUE rb_string_from_hb_string(hb_string_T string) { VALUE rb_token_from_c_struct(token_T* token) { if (!token) { return Qnil; } - VALUE value = rb_string_from_hb_string(token->value); - VALUE range = rb_range_from_c_struct(token->range); - VALUE location = rb_location_from_c_struct(token->location); - VALUE type = rb_string_from_hb_string(token_type_to_string(token->type)); + init_ast_value_ivar_ids(); - VALUE args[4] = { value, range, location, type }; + VALUE obj = rb_obj_alloc(cToken); + rb_ivar_set(obj, id_value, rb_string_from_hb_string(token->value)); + rb_ivar_set(obj, id_range, rb_range_from_c_struct(token->range)); + rb_ivar_set(obj, id_location, rb_location_from_c_struct(token->location)); + rb_ivar_set(obj, id_type, rb_string_from_hb_string(token_type_to_string(token->type))); - return rb_class_new_instance(4, args, cToken); + return obj; } VALUE create_lex_result(hb_array_T* tokens, VALUE source) { diff --git a/ext/herb/extension_helpers.h b/ext/herb/extension_helpers.h index 34d3c5f04..0830a6e99 100644 --- a/ext/herb/extension_helpers.h +++ b/ext/herb/extension_helpers.h @@ -3,12 +3,19 @@ #include +#include + #include "../../src/include/herb.h" #include "../../src/include/lexer/token.h" #include "../../src/include/location/location.h" #include "../../src/include/location/position.h" #include "../../src/include/location/range.h" +// When false, Herb's Ruby AST is built without Location/Range/Position objects +// (they are left nil). Set per-parse by Herb_parse from the `track_locations` +// option. Only affects Ruby object materialization, not the native parse. +extern bool herb_ext_track_locations; + const char* check_string(VALUE value); VALUE rb_string_from_hb_string(hb_string_T string); From e6c29c573023a1e209b2e154c4fcfdfa827bf16c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 24 Jul 2026 15:00:38 -0600 Subject: [PATCH 2/6] Intern AST node/token type strings and short token values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every node materialized a fresh String for its type (e.g. "AST_HTML_ELEMENT_NODE"), and every token a fresh String for its type and value — even though these are drawn from tiny fixed vocabularies. In a typical template ~96% of token values are duplicates of a handful of structural strings ("\n", "%>", "<%", ">", " ", tag names, ...). Intern node and token type strings, and token values up to 16 bytes, so the whole AST shares one frozen String per distinct value. Longer token values (arbitrary text content) are left as ordinary strings since they rarely repeat. This roughly halves the number of strings allocated during a parse. --- ext/herb/extension_helpers.c | 35 ++++++++++++++++++++++++++++++++-- ext/herb/extension_helpers.h | 1 + templates/ext/herb/nodes.c.erb | 2 +- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/ext/herb/extension_helpers.c b/ext/herb/extension_helpers.c index 0387fce25..45d643852 100644 --- a/ext/herb/extension_helpers.c +++ b/ext/herb/extension_helpers.c @@ -2,6 +2,8 @@ #include +#include + #include "extension.h" #include "extension_helpers.h" #include "nodes.h" @@ -23,6 +25,11 @@ const char* check_string(VALUE value) { return RSTRING_PTR(value); } +// Maximum byte length of a token value we will intern. Structural tokens and +// identifiers (delimiters, tag/attribute names) are short and highly repeated; +// longer values are treated as unique text content and not interned. +#define HERB_INTERN_VALUE_MAX_LENGTH 16 + // Whether to materialize source Location/Range/Position objects while building // the Ruby AST. For rendering (no validation, no debug), locations are never // read, yet they account for ~50% of parse allocations and ~28% of parse time @@ -96,16 +103,40 @@ VALUE rb_string_from_hb_string(hb_string_T string) { return rb_utf8_str_new(string.data, string.length); } +// Like rb_string_from_hb_string, but returns a deduplicated frozen (interned) +// String. Use only for values drawn from a small fixed set — e.g. token/node +// type identifiers — so the whole AST shares one String per distinct value +// instead of allocating a fresh copy each time. +VALUE rb_interned_string_from_hb_string(hb_string_T string) { + if (hb_string_is_null(string)) { return Qnil; } + + return rb_enc_interned_str(string.data, string.length, rb_utf8_encoding()); +} + VALUE rb_token_from_c_struct(token_T* token) { if (!token) { return Qnil; } init_ast_value_ivar_ids(); VALUE obj = rb_obj_alloc(cToken); - rb_ivar_set(obj, id_value, rb_string_from_hb_string(token->value)); + // Token values are overwhelmingly drawn from a tiny structural vocabulary + // ("\n", "%>", "<%", ">", " ", "\"", tag names, etc.) — in practice ~96% are + // duplicates. Intern short values so the whole token stream shares one frozen + // String per distinct value instead of allocating a fresh copy per token. + // Longer values (arbitrary text content) are left as ordinary strings: they + // rarely repeat, so interning them would only pollute the fstring table. + hb_string_T value = token->value; + if (!hb_string_is_null(value) && value.length <= HERB_INTERN_VALUE_MAX_LENGTH) { + rb_ivar_set(obj, id_value, rb_interned_string_from_hb_string(value)); + } else { + rb_ivar_set(obj, id_value, rb_string_from_hb_string(value)); + } rb_ivar_set(obj, id_range, rb_range_from_c_struct(token->range)); rb_ivar_set(obj, id_location, rb_location_from_c_struct(token->location)); - rb_ivar_set(obj, id_type, rb_string_from_hb_string(token_type_to_string(token->type))); + // A token's type is one of a small fixed set of identifier strings. Interning + // them (deduplicated frozen strings) means the whole token stream shares one + // String object per type instead of allocating a fresh copy per token. + rb_ivar_set(obj, id_type, rb_interned_string_from_hb_string(token_type_to_string(token->type))); return obj; } diff --git a/ext/herb/extension_helpers.h b/ext/herb/extension_helpers.h index 0830a6e99..c6c1a8256 100644 --- a/ext/herb/extension_helpers.h +++ b/ext/herb/extension_helpers.h @@ -18,6 +18,7 @@ extern bool herb_ext_track_locations; const char* check_string(VALUE value); VALUE rb_string_from_hb_string(hb_string_T string); +VALUE rb_interned_string_from_hb_string(hb_string_T string); VALUE rb_position_from_c_struct(position_T position); VALUE rb_location_from_c_struct(location_T location); diff --git a/templates/ext/herb/nodes.c.erb b/templates/ext/herb/nodes.c.erb index ee0420c26..402ef29b0 100644 --- a/templates/ext/herb/nodes.c.erb +++ b/templates/ext/herb/nodes.c.erb @@ -32,7 +32,7 @@ static VALUE rb_<%= node.human %>_from_c_struct(<%= node.struct_type %>* <%= nod AST_NODE_T* node = &<%= node.human %>->base; - VALUE type = rb_string_from_hb_string(ast_node_type_to_string(node)); + VALUE type = rb_interned_string_from_hb_string(ast_node_type_to_string(node)); VALUE location = rb_location_from_c_struct(node->location); VALUE errors = rb_errors_array_from_c_array(node->errors); From 935ae384f462b0835b46692bcd40392f101f6570 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 24 Jul 2026 15:01:47 -0600 Subject: [PATCH 3/6] Skip recursive error walk for cleanly-parsed templates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParseResult#errors collects errors by walking the entire AST (value.recursive_errors) on every call, allocating along the way — even when the template parsed with no errors, which is the overwhelmingly common case. Count errors as they are materialized onto nodes (rb_errors_array_from_c_array) and record the total on the ParseResult as @total_error_count. When it is zero, ParseResult#errors returns the top-level errors directly and skips the full recursive walk entirely. --- ext/herb/extension.c | 10 +++++++++- ext/herb/extension_helpers.c | 4 ++++ ext/herb/extension_helpers.h | 6 ++++++ lib/herb/parse_result.rb | 5 +++++ templates/ext/herb/error_helpers.c.erb | 3 +++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ext/herb/extension.c b/ext/herb/extension.c index b1bc23417..81002f8b5 100644 --- a/ext/herb/extension.c +++ b/ext/herb/extension.c @@ -45,7 +45,15 @@ static VALUE parse_convert_body(VALUE arg) { // the native parse released the GVL. herb_ext_track_locations = args->track_locations; - return create_parse_result(args->root, args->source, args->parser_options); + // Reset the per-parse error counter, then record the total on the result so + // Ruby can skip the recursive error walk when the template parsed cleanly. + herb_ext_error_count = 0; + + VALUE result = create_parse_result(args->root, args->source, args->parser_options); + + rb_ivar_set(result, rb_intern("@total_error_count"), UINT2NUM(herb_ext_error_count)); + + return result; } static VALUE parse_cleanup(VALUE arg) { diff --git a/ext/herb/extension_helpers.c b/ext/herb/extension_helpers.c index 45d643852..2e45a9803 100644 --- a/ext/herb/extension_helpers.c +++ b/ext/herb/extension_helpers.c @@ -39,6 +39,10 @@ const char* check_string(VALUE value) { // the GVL, so no two parses materialize concurrently. bool herb_ext_track_locations = true; +// Accumulates the number of errors attached to AST nodes during the current +// parse's materialization (see rb_errors_array_from_c_array). Reset per parse. +uint32_t herb_ext_error_count = 0; + // Cached instance-variable IDs for the hot AST value objects (Position, // Location, Range, Token). Building these via rb_obj_alloc + rb_ivar_set sets // the ivars directly, mirroring the Ruby classes' initializers. diff --git a/ext/herb/extension_helpers.h b/ext/herb/extension_helpers.h index c6c1a8256..7f6471f10 100644 --- a/ext/herb/extension_helpers.h +++ b/ext/herb/extension_helpers.h @@ -16,6 +16,12 @@ // option. Only affects Ruby object materialization, not the native parse. extern bool herb_ext_track_locations; +// Total number of errors materialized onto AST nodes during the current parse. +// Accumulated by rb_errors_array_from_c_array as the tree is built and read by +// Herb_parse to record a total on the ParseResult, letting Ruby skip the full +// recursive error walk when a template parsed cleanly. +extern uint32_t herb_ext_error_count; + const char* check_string(VALUE value); VALUE rb_string_from_hb_string(hb_string_T string); VALUE rb_interned_string_from_hb_string(hb_string_T string); diff --git a/lib/herb/parse_result.rb b/lib/herb/parse_result.rb index 5a335bcad..f55732bbc 100644 --- a/lib/herb/parse_result.rb +++ b/lib/herb/parse_result.rb @@ -24,6 +24,11 @@ def initialize(value, source, warnings, errors, options) #: () -> Array[Herb::Errors::Error] def errors + # The native extension records the total number of errors materialized + # onto the AST during parsing. When it is zero we can skip the recursive + # walk of every node entirely — the common case for valid templates. + return super if defined?(@total_error_count) && @total_error_count.zero? + super + value.recursive_errors end diff --git a/templates/ext/herb/error_helpers.c.erb b/templates/ext/herb/error_helpers.c.erb index ff8c9c088..b47f01afe 100644 --- a/templates/ext/herb/error_helpers.c.erb +++ b/templates/ext/herb/error_helpers.c.erb @@ -90,6 +90,9 @@ VALUE rb_errors_array_from_c_array(hb_array_T* array) { if (child_node) { VALUE rb_child = rb_error_from_c_struct(child_node); rb_ary_push(rb_array, rb_child); + // Track total errors materialized so callers can skip the recursive + // error walk when a template parsed cleanly. + herb_ext_error_count++; } } } From deee5d67a20bdd0e2c3e14b48a597f4e016fed32 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 24 Jul 2026 15:01:59 -0600 Subject: [PATCH 4/6] Collect recursive errors into a shared accumulator Node#recursive_errors was `errors + compact_child_nodes.flat_map(&:recursive_errors)`, which allocated an intermediate array (and a compacted child array) at every node. Rewrite it to walk children iteratively into a single shared accumulator, avoiding the per-node throwaway allocations on large trees. --- lib/herb/ast/node.rb | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/herb/ast/node.rb b/lib/herb/ast/node.rb index f7a06684f..e49370f8a 100644 --- a/lib/herb/ast/node.rb +++ b/lib/herb/ast/node.rb @@ -116,8 +116,25 @@ def compact_child_nodes end #: () -> Array[Herb::Errors::Error] - def recursive_errors - errors + compact_child_nodes.flat_map(&:recursive_errors) + def recursive_errors(accumulator = []) + accumulator.concat(errors) unless errors.empty? + + # Walk children iteratively into a single shared accumulator rather than + # allocating an intermediate array at every node (compact + flat_map + + # concat). This is a hot path: it runs for every node on every parse, so + # for large error-free trees the previous approach allocated several + # throwaway arrays per node. + children = child_nodes + index = 0 + count = children.size + + while index < count + child = children[index] + child&.recursive_errors(accumulator) + index += 1 + end + + accumulator end end end From 534bf3ece370b11fa42b938a093ab654589eaabb Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 24 Jul 2026 15:02:07 -0600 Subject: [PATCH 5/6] Compute Engine#relative_file_path lazily Engine#initialize eagerly built two Pathnames and ran Pathname#relative_path_from + #to_s on every compile, but relative_file_path is only consulted when emitting errors, overlays, or debug output. Derive it on demand in a reader instead, keeping it off the common, error-free render path. --- lib/herb/engine.rb | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/lib/herb/engine.rb b/lib/herb/engine.rb index db193a916..9720cb73c 100644 --- a/lib/herb/engine.rb +++ b/lib/herb/engine.rb @@ -18,7 +18,7 @@ module Herb class Engine - attr_reader :src, :filename, :project_path, :relative_file_path, :bufvar, :debug, :content_for_head, + attr_reader :src, :filename, :project_path, :bufvar, :debug, :content_for_head, :validation_error_template, :visitors, :enabled_validators # @rbs! @@ -59,12 +59,10 @@ def initialize(input, properties = {}) @filename = properties[:filename] ? ::Pathname.new(properties[:filename]) : nil @project_path = ::Pathname.new(properties[:project_path] || Dir.pwd) - if @filename - absolute_filename = @filename.absolute? ? @filename : @project_path + @filename - @relative_file_path = absolute_filename.relative_path_from(@project_path).to_s - else - @relative_file_path = "unknown" - end + # @relative_file_path is only referenced when emitting errors, overlays, + # or debug output. Computing it eagerly runs Pathname#relative_path_from + # (and a #to_s) on every compile, even for valid templates that never use + # it, so it is derived lazily in #relative_file_path instead. @bufvar = properties[:bufvar] || properties[:outvar] || "_buf" @escape = properties.fetch(:escape) { properties.fetch(:escape_html, false) } @@ -184,6 +182,19 @@ def initialize(input, properties = {}) freeze end + # Path of the template relative to the project root, used for error, + # overlay, and debug output. Derived on demand rather than in #initialize + # because Pathname#relative_path_from is comparatively expensive and is + # unnecessary on the common, error-free render path. Not memoized: the + # engine freezes itself after compiling, and this is only consulted on the + # rare error/debug paths, so recomputation is immaterial. + def relative_file_path + return "unknown" unless @filename + + absolute_filename = @filename.absolute? ? @filename : @project_path + @filename + absolute_filename.relative_path_from(@project_path).to_s + end + def self.h(value) value.to_s.gsub(/[&<>"']/, ESCAPE_TABLE) end @@ -464,7 +475,7 @@ def add_validation_overlay(validators, input = nil) column = location&.start&.column || 0 source = input || @src - overlay_generator = ValidationErrorOverlay.new(source, error, filename: @relative_file_path) + overlay_generator = ValidationErrorOverlay.new(source, error, filename: relative_file_path) html_fragment = overlay_generator.generate_fragment escaped_message = escape_attr(error[:message]) @@ -478,7 +489,7 @@ def add_validation_overlay(validators, input = nil) data-code="#{error[:code]}" data-line="#{line}" data-column="#{column}" - data-filename="#{escape_attr(@relative_file_path)}" + data-filename="#{escape_attr(relative_file_path)}" data-message="#{escaped_message}" #{"data-suggestion=\"#{escaped_suggestion}\"" if error[:suggestion]} data-timestamp="#{Time.now.utc.iso8601}" @@ -507,7 +518,7 @@ def add_parser_error_overlay(parser_errors, input) overlay_generator = ParserErrorOverlay.new( input, parser_errors, - filename: @relative_file_path + filename: relative_file_path ) error_html = overlay_generator.generate_html From 04afe67e9d96c85a7c197ede949f179c1c01b38b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 24 Jul 2026 15:02:17 -0600 Subject: [PATCH 6/6] Fuse whitespace compaction into optimize_tokens; use String#match? optimize_tokens ran compact_whitespace_tokens first, which built a whole intermediate array (map.with_index + compact), then re-scanned it to merge adjacent text. Fold whitespace resolution into optimize_tokens' single pass: whitespace is resolved against its neighbours in the original stream and text is merged inline, removing an array allocation and a full pass per template. Also switch the boolean-context regexp guards in the whitespace helpers from =~ to String#match?, which is faster and does not allocate MatchData or set $~. --- lib/herb/engine/compiler.rb | 74 ++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/lib/herb/engine/compiler.rb b/lib/herb/engine/compiler.rb index bdf551baa..30f478405 100644 --- a/lib/herb/engine/compiler.rb +++ b/lib/herb/engine/compiler.rb @@ -469,46 +469,60 @@ def add_expression_escaped(code) def optimize_tokens(tokens) return tokens if tokens.empty? - compacted = compact_whitespace_tokens(tokens) - optimized = [] #: Array[untyped] - current_text = "" + current_text = nil current_context = nil - compacted.each do |type, value, context, escaped| + # Single pass over the raw token stream. Whitespace tokens are resolved + # against their neighbours in the ORIGINAL stream (dropped, or turned + # into text) and consecutive text is merged into one buffer inline. This + # replaces the former two-pass approach (compact_whitespace_tokens built + # a whole intermediate array via map.with_index + compact, which this + # loop then re-scanned), saving an array allocation and a full pass per + # template. + tokens.each_with_index do |token, index| + type = token[0] + + if type == :whitespace + next if adjacent_whitespace?(tokens, index) + next if whitespace_before_code_sequence?(tokens, index) + + # Surviving whitespace becomes plain text and joins the text run. + type = :text + end + if type == :text - current_text += value - current_context ||= context + value = token[1] + + if current_text + # Mutate a single buffer instead of `current_text += value`, which + # reallocated and copied the whole accumulated string on every + # text token (quadratic for long runs of adjacent text). + current_text << value + current_context ||= token[2] + else + # Start a fresh buffer; dup so we never mutate the token's own + # (possibly frozen) value string. + current_text = value.dup + current_context = token[2] + end else - unless current_text.empty? + if current_text optimized << [:text, current_text, current_context] - current_text = "" + current_text = nil current_context = nil end - optimized << [type, value, context, escaped] + optimized << [type, token[1], token[2], token[3]] end end - optimized << [:text, current_text, current_context] unless current_text.empty? + optimized << [:text, current_text, current_context] if current_text optimized end - def compact_whitespace_tokens(tokens) - return tokens if tokens.empty? - - tokens.map.with_index { |token, index| - next token unless token[0] == :whitespace - - next nil if adjacent_whitespace?(tokens, index) - next nil if whitespace_before_code_sequence?(tokens, index) - - [:text, token[1], token[2]] - }.compact - end - def adjacent_whitespace?(tokens, index) prev_token = index.positive? ? tokens[index - 1] : nil next_token = index < tokens.length - 1 ? tokens[index + 1] : nil @@ -519,11 +533,11 @@ def adjacent_whitespace?(tokens, index) def trailing_whitespace?(token) return false unless token - token[0] == :whitespace || (token[0] == :text && token[1] =~ /\s\z/) + token[0] == :whitespace || (token[0] == :text && token[1].match?(/\s\z/)) end def leading_whitespace?(token) - token && token[0] == :text && token[1] =~ /\A\s/ + token && token[0] == :text && token[1].match?(/\A\s/) end def whitespace_before_code_sequence?(tokens, current_index) @@ -589,7 +603,7 @@ def at_line_start? last_value = @tokens.last[1] if last_type == :text - last_value.empty? || last_value.end_with?("\n") || (last_value =~ WHITESPACE_ONLY && preceding_token_ends_with_newline?) || last_value =~ TRAILING_INDENTATION + last_value.empty? || last_value.end_with?("\n") || (last_value.match?(WHITESPACE_ONLY) && preceding_token_ends_with_newline?) || last_value.match?(TRAILING_INDENTATION) elsif EXPRESSION_TOKEN_TYPES.include?(last_type) @last_trim_consumed_newline else @@ -651,9 +665,9 @@ def extract_and_remove_leading_space! text = @tokens.last[1] - if text =~ TRAILING_INDENTATION + if text.match?(TRAILING_INDENTATION) text.sub!(TRAILING_WHITESPACE, "") - elsif text =~ WHITESPACE_ONLY + elsif text.match?(WHITESPACE_ONLY) text.replace("") end @@ -698,10 +712,10 @@ def remove_trailing_whitespace_from_last_token! text = token[1] removed = text[TRAILING_WHITESPACE] || "" - if text =~ TRAILING_INDENTATION + if text.match?(TRAILING_INDENTATION) text.sub!(TRAILING_WHITESPACE, "") token[1] = text - elsif text =~ WHITESPACE_ONLY + elsif text.match?(WHITESPACE_ONLY) text.replace("") token[1] = text end