diff --git a/ext/herb/extension.c b/ext/herb/extension.c index 0d59d68a6..81002f8b5 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,7 +40,20 @@ typedef struct { static VALUE parse_convert_body(VALUE arg) { parse_args_T* args = (parse_args_T*) arg; - return create_parse_result(args->root, args->source, args->parser_options); + // 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; + + // 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) { @@ -194,9 +208,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..2e45a9803 100644 --- a/ext/herb/extension_helpers.c +++ b/ext/herb/extension_helpers.c @@ -1,5 +1,9 @@ #include +#include + +#include + #include "extension.h" #include "extension_helpers.h" #include "nodes.h" @@ -21,28 +25,80 @@ 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 +// (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; + +// 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. +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(); - return rb_class_new_instance(2, args, cPosition); + 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 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; } + + init_ast_value_ivar_ids(); - return rb_class_new_instance(2, args, cLocation); + 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(); - return rb_class_new_instance(2, args, cRange); + 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 obj; } VALUE rb_string_from_hb_string(hb_string_T string) { @@ -51,17 +107,42 @@ VALUE rb_string_from_hb_string(hb_string_T string) { return rb_utf8_str_new(string.data, string.length); } -VALUE rb_token_from_c_struct(token_T* token) { - if (!token) { return Qnil; } +// 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; } - 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)); + return rb_enc_interned_str(string.data, string.length, rb_utf8_encoding()); +} - VALUE args[4] = { value, range, location, type }; +VALUE rb_token_from_c_struct(token_T* token) { + if (!token) { return Qnil; } - return rb_class_new_instance(4, args, cToken); + init_ast_value_ivar_ids(); + + VALUE obj = rb_obj_alloc(cToken); + // 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)); + // 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; } 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..7f6471f10 100644 --- a/ext/herb/extension_helpers.h +++ b/ext/herb/extension_helpers.h @@ -3,14 +3,28 @@ #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; + +// 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); VALUE rb_position_from_c_struct(position_T position); VALUE rb_location_from_c_struct(location_T location); 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 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 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 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++; } } } 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);