From 81d25460c536cf7e50358281023bc59e035feffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Fri, 3 Jul 2026 21:50:34 +0200 Subject: [PATCH 1/2] Eager-load Action View templates into the resolver cache Add FileSystemResolver#eager_load_templates, which globs the view path once and populates the unbound-template cache up front instead of lazily on first lookup. The cache stays mutable, so binding new locals at render time keeps working exactly as before; this only warms the cache. Also extract the lookup out of #_find_all into #unbound_templates_for and drop the throwaway Concurrent::Map that was allocated on every uncached lookup, computing directly instead. --- .../lib/action_view/template/resolver.rb | 27 ++++++++++++----- .../template/file_system_resolver_test.rb | 29 +++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index f07f97d568fdd..7b649dd80d7bf 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -104,6 +104,13 @@ def clear_cache super end + def eager_load_templates + template_glob("**/*").each do |file| + unbound = build_unbound_template(file) + (@unbound_templates[unbound.virtual_path] ||= []) << unbound + end + end + def to_s @path.to_s end @@ -130,19 +137,25 @@ def built_templates # :nodoc: private def _find_all(name, prefix, partial, details, key, locals) requested_details = key || TemplateDetails::Requested.new(**details) - cache = key ? @unbound_templates : Concurrent::Map.new - - unbound_templates = - cache.compute_if_absent(TemplatePath.virtual(name, prefix, partial)) do - path = TemplatePath.build(name, prefix, partial) - unbound_templates_from_path(path) - end + unbound_templates = unbound_templates_for(name, prefix, partial, !key.nil?) filter_and_sort_by_details(unbound_templates, requested_details).map do |unbound_template| unbound_template.bind_locals(locals) end end + def unbound_templates_for(name, prefix, partial, cache) + virtual = TemplatePath.virtual(name, prefix, partial) + + if cache + @unbound_templates.compute_if_absent(virtual) do + unbound_templates_from_path(TemplatePath.build(name, prefix, partial)) + end + else + unbound_templates_from_path(TemplatePath.build(name, prefix, partial)) + end + end + def source_for_template(template) Template::Sources::File.new(template) end diff --git a/actionview/test/template/file_system_resolver_test.rb b/actionview/test/template/file_system_resolver_test.rb index aa03fdcb1383d..1729f03343f05 100644 --- a/actionview/test/template/file_system_resolver_test.rb +++ b/actionview/test/template/file_system_resolver_test.rb @@ -9,4 +9,33 @@ class FileSystemResolverTest < ActiveSupport::TestCase def resolver ActionView::FileSystemResolver.new(tmpdir) end + + DETAILS = { locale: [:en], formats: [:html], variants: [], handlers: [:erb] }.freeze + + def find_all(resolver, name = "hello_world", prefix = "test", partial = false, locals = []) + resolver.find_all(name, prefix, partial, DETAILS, nil, locals) + end + + def test_eager_load_templates_populates_cache_without_freezing + with_file "test/hello_world.html.erb", "Hello!" + r = resolver + r.eager_load_templates + + assert_not r.frozen? + templates = find_all(r) + assert_equal 1, templates.size + assert_equal "Hello!", templates[0].source + end + + def test_eager_loaded_resolver_still_binds_new_locals + with_file "test/hello_world.html.erb", "<%= message %>" + r = resolver + r.eager_load_templates + + a = find_all(r, "hello_world", "test", false, [:message])[0] + b = find_all(r, "hello_world", "test", false, [:message, :other])[0] + + assert_not_same a, b + assert_not r.frozen? + end end From 0a6751b53946611ca6a3a3ce9f64158376a57ce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Fri, 3 Jul 2026 21:50:50 +0200 Subject: [PATCH 2/2] Make Action View template objects shareable when frozen Give Template, UnboundTemplate, the file source, and the resolver's PathParser a #freeze that turns them into deeply-frozen, Ractor-shareable objects, and add FileSystemResolver#freeze to freeze the whole eager-built cache. Combined with #eager_load_templates, a resolver can be built and then frozen so its templates can be shared across Ractors. Template#freeze memoizes the method name, strict-locals declaration and compiled source, then drops the compile mutex. UnboundTemplate collapses to a single @strict_locals_template once it discovers the template is strict, rather than a defaulting Hash, which makes freezing straightforward. Freezing requires strict locals: a non-strict template compiles a distinct method per set of locals, so it can't be reduced to one shareable template. UnboundTemplate#freeze therefore raises for any template that hasn't declared its locals. Support for freezing non-strict templates and partials is left to a follow-up. --- actionview/lib/action_view/template.rb | 13 ++++++ .../lib/action_view/template/resolver.rb | 28 ++++++++++-- .../lib/action_view/template/sources/file.rb | 5 +++ .../lib/action_view/unbound_template.rb | 44 ++++++++++++++----- .../template/file_system_resolver_test.rb | 43 ++++++++++++++++++ 5 files changed, 118 insertions(+), 15 deletions(-) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index c5836bf4a20b2..9efc306a2a0da 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -408,6 +408,19 @@ def strict_locals? strict_locals! end + def freeze # :nodoc: + strict_locals! + method_name.freeze + @source.freeze + @identifier.freeze + @virtual_path&.freeze + @locals&.freeze + @strict_locals.freeze if @strict_locals.is_a?(String) + @variant.freeze if @variant.is_a?(String) + @compile_mutex = nil + super + end + # Exceptions are marshalled when using the parallel test runner with DRb, so we need # to ensure that references to the template object can be marshalled as well. This means forgoing # the marshalling of the compiler mutex and instantiating that again on unmarshalling. diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 7b649dd80d7bf..4c1bc0ecdbeff 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -33,9 +33,12 @@ def build_path_regex }x end - def parse(path) + def path_regex @regex ||= build_path_regex - match = @regex.match(path) + end + + def parse(path) + match = path_regex.match(path) path = TemplatePath.build(match[:action], match[:prefix] || "", !!match[:partial]) details = TemplateDetails.new( match[:locale]&.to_sym, @@ -45,6 +48,11 @@ def parse(path) ) ParsedPath.new(path, details) end + + def freeze + path_regex + super + end end cattr_accessor :caching, default: true @@ -111,6 +119,18 @@ def eager_load_templates end end + def freeze + @path.freeze + @path_parser.freeze + @unbound_templates = @unbound_templates.each_pair.to_h unless @unbound_templates.is_a?(::Hash) + @unbound_templates.each_value do |unbound_templates| + unbound_templates.each(&:freeze) + unbound_templates.freeze + end + @unbound_templates.freeze + super + end + def to_s @path.to_s end @@ -147,7 +167,9 @@ def _find_all(name, prefix, partial, details, key, locals) def unbound_templates_for(name, prefix, partial, cache) virtual = TemplatePath.virtual(name, prefix, partial) - if cache + if frozen? + @unbound_templates[virtual] || [].freeze + elsif cache @unbound_templates.compute_if_absent(virtual) do unbound_templates_from_path(TemplatePath.build(name, prefix, partial)) end diff --git a/actionview/lib/action_view/template/sources/file.rb b/actionview/lib/action_view/template/sources/file.rb index 521bd06f45f08..028f27c3fb27d 100644 --- a/actionview/lib/action_view/template/sources/file.rb +++ b/actionview/lib/action_view/template/sources/file.rb @@ -11,6 +11,11 @@ def initialize(filename) def to_s ::File.binread @filename end + + def freeze + @filename.freeze + super + end end end end diff --git a/actionview/lib/action_view/unbound_template.rb b/actionview/lib/action_view/unbound_template.rb index b9e28e2e7df71..a6fb74801ce2d 100644 --- a/actionview/lib/action_view/unbound_template.rb +++ b/actionview/lib/action_view/unbound_template.rb @@ -13,13 +13,41 @@ def initialize(source, identifier, details:, virtual_path:) @details = details @virtual_path = virtual_path + # Strict templates ignore the locals passed at render time, so a single + # template serves every key. @strict_locals_template holds that one + # template once we've discovered the template is strict; until then + # @templates caches one template per set of locals. + @strict_locals_template = nil @templates = Concurrent::Map.new(initial_capacity: 2) @write_lock = Mutex.new end def bind_locals(locals) - unless template = @templates[locals] + @strict_locals_template || @templates[locals] || build_bound_template(locals) + end + + def built_templates # :nodoc: + @strict_locals_template ? [@strict_locals_template] : @templates.values + end + + def freeze # :nodoc: + unless bind_locals([]).strict_locals? + raise ArgumentError, "Cannot freeze #{@virtual_path.inspect}: templates must declare strict locals (e.g. `<%# locals: () %>`) to be frozen." + end + @source.freeze + @identifier.freeze + @virtual_path.freeze + @details.freeze + @strict_locals_template.freeze + @templates = nil + @write_lock = nil + super + end + + private + def build_bound_template(locals) @write_lock.synchronize do + return @strict_locals_template if @strict_locals_template normalized_locals = normalize_locals(locals) # We need ||=, both to dedup on the normalized locals and to check @@ -27,25 +55,17 @@ def bind_locals(locals) template = (@templates[normalized_locals] ||= build_template(normalized_locals)) if template.strict_locals? - # Under strict locals, we only need one template. - # This replaces the @templates Concurrent::Map with a hash which - # returns this template for every key. - @templates = Hash.new(template).freeze + @strict_locals_template = template else # This may have already been assigned, but we've already de-dup'd so # reassignment is fine. @templates[locals.dup] = template end + + template end end - template - end - def built_templates # :nodoc: - @templates.values - end - - private def build_template(locals) Template.new( @source, diff --git a/actionview/test/template/file_system_resolver_test.rb b/actionview/test/template/file_system_resolver_test.rb index 1729f03343f05..5fe282d51b95a 100644 --- a/actionview/test/template/file_system_resolver_test.rb +++ b/actionview/test/template/file_system_resolver_test.rb @@ -38,4 +38,47 @@ def test_eager_loaded_resolver_still_binds_new_locals assert_not_same a, b assert_not r.frozen? end + + def test_freeze_after_eager_load_makes_resolver_shareable + with_file "test/hello_world.html.erb", "<%# locals: () %>Hi" + r = resolver + r.eager_load_templates + r.freeze + + assert_predicate r, :frozen? + assert Ractor.shareable?(r) + + templates = find_all(r) + assert_equal 1, templates.size + assert_predicate templates[0], :frozen? + end + + def test_freeze_raises_for_non_strict_partial + with_file "test/_card.html.erb", "<%= post %>" + r = resolver + r.eager_load_templates + + error = assert_raises(ArgumentError) { r.freeze } + assert_match "test/_card", error.message + assert_match "strict locals", error.message + end + + def test_freeze_raises_for_non_strict_template + with_file "test/hello_world.html.erb", "no locals here" + r = resolver + r.eager_load_templates + + error = assert_raises(ArgumentError) { r.freeze } + assert_match "test/hello_world", error.message + assert_match "strict locals", error.message + end + + def test_frozen_resolver_returns_empty_for_missing_template + with_file "test/hello_world.html.erb", "<%# locals: () %>Hi" + r = resolver + r.eager_load_templates + r.freeze + + assert_empty find_all(r, "nonexistent") + end end