From c6ce8e67c5d78a566678ebeed9d2336e07819585 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 3 Nov 2026 10:37:32 -0700 Subject: [PATCH] Share filesystem work during cache digest computation --- docs/CHANGELOG.md | 4 ++ lib/view_component/base.rb | 8 +-- .../cache_digest/digestor_lookup.rb | 18 +++--- lib/view_component/cache_digest/resolver.rb | 8 ++- lib/view_component/compile_cache.rb | 31 +++++----- test/sandbox/test/base_test.rb | 29 ++++++++++ test/sandbox/test/cacheable_test.rb | 56 +++++++++++++++++++ 7 files changed, 125 insertions(+), 29 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 954223c06..dfac93bd3 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Share sidecar directory listings and source reads during cold cache digests, and hash shared Ruby and sidecar files once per digest computation or boot-time precomputation. + + *Joel Hawksley* + * Avoid missing-template digest errors for unresolvable interpolated render paths in components and included helpers. *Joel Hawksley* diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index ef6b07b02..2d0f8a705 100644 --- a/lib/view_component/base.rb +++ b/lib/view_component/base.rb @@ -667,10 +667,10 @@ def sidecar_files(extensions) (sidecar_files - [identifier] + sidecar_directory_files + nested_component_files).uniq end - # `sidecar_files`, looked up once per component while eager loading: - # every component looks up its superclasses' templates and translations - # too, when it compiles, loads its translations, and computes its cache - # digest. + # `sidecar_files`, looked up once per component during eager loading or a + # digest computation: every component looks up its superclasses' templates + # and translations too, when it compiles, loads its translations, and + # computes its cache digest. # # @private def __vc_sidecar_files(extensions) diff --git a/lib/view_component/cache_digest/digestor_lookup.rb b/lib/view_component/cache_digest/digestor_lookup.rb index 94b3c6499..f629152cb 100644 --- a/lib/view_component/cache_digest/digestor_lookup.rb +++ b/lib/view_component/cache_digest/digestor_lookup.rb @@ -46,16 +46,18 @@ def digest(finder:, **) # everything it renders, which for components rendering a shared helper # module is most of the application. def tree(name, finder, partial = false, seen = {}) - pruned = CacheDigest.memo&.fetch(:pruned, nil) - cached = finder.digest_cache[name] if pruned && partial - return super unless cached + CompileCache.with_sidecar_files_cache do + pruned = CacheDigest.memo&.fetch(:pruned, nil) + cached = finder.digest_cache[name] if pruned && partial + next super unless cached - path = ActionView::TemplatePath.parse(name) - template = find_template(finder, path.name, [path.prefix], partial, []) - return super unless template + path = ActionView::TemplatePath.parse(name) + template = find_template(finder, path.name, [path.prefix], partial, []) + next super unless template - pruned[name] = cached - seen[template.identifier] ||= ActionView::Digestor::Node.create(name, name.gsub(%r{/_}, "/"), template, partial) + pruned[name] = cached + seen[template.identifier] ||= ActionView::Digestor::Node.create(name, name.gsub(%r{/_}, "/"), template, partial) + end end private diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index b33283ece..5b03b8bb2 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -160,8 +160,10 @@ def source_files(component) end def ruby_files(component) - component_ancestors(component).map(&:identifier).concat(module_files(component)) - .compact.uniq.select { |path| ::File.exist?(path) } + CacheDigest.memoize([:ruby_files, component]) do + component_ancestors(component).map(&:identifier).concat(module_files(component)) + .compact.uniq.select { |path| ::File.exist?(path) } + end end # Sidecar template files plus inline templates, which live in the Ruby @@ -243,7 +245,7 @@ def ruby_sources(component) end def file_digest(path) - ActiveSupport::Digest.hexdigest(::File.binread(path)) + CacheDigest.memoize([:file_digest, path]) { ActiveSupport::Digest.hexdigest(::File.binread(path)) } end # Built once at load time rather than memoized, so no class-level state diff --git a/lib/view_component/compile_cache.rb b/lib/view_component/compile_cache.rb index 41ed13e09..6b17b8663 100644 --- a/lib/view_component/compile_cache.rb +++ b/lib/view_component/compile_cache.rb @@ -26,25 +26,28 @@ def invalidate! cache.clear end - # Source files can't change while the application eager loads: share - # directory listings, sidecar lookups, and file contents within the block. - # Fiber storage keeps the caches from surviving boot. + # Share directory listings, sidecar lookups, and file contents within one + # eager load or digest computation, but never across reloads. def with_sidecar_files_cache - Fiber[:__vc_sidecar_files] = {} - Fiber[:__vc_sidecar_directories] = {} - Fiber[:__vc_source_files] = {} - Fiber[:__vc_path_parser] = ActionView::Resolver::PathParser.new - yield - ensure - Fiber[:__vc_sidecar_files] = nil - Fiber[:__vc_sidecar_directories] = nil - Fiber[:__vc_source_files] = nil - Fiber[:__vc_path_parser] = nil + return yield if Fiber[:__vc_sidecar_files] + + begin + Fiber[:__vc_sidecar_files] = {} + Fiber[:__vc_sidecar_directories] = {} + Fiber[:__vc_source_files] = {} + Fiber[:__vc_path_parser] = ActionView::Resolver::PathParser.new + yield + ensure + Fiber[:__vc_sidecar_files] = nil + Fiber[:__vc_sidecar_directories] = nil + Fiber[:__vc_source_files] = nil + Fiber[:__vc_path_parser] = nil + end end def path_parser # Its regular expression depends on registered handlers, formats and locales. - # Reuse it during eager loading, but not across reloads. + # Reuse it during eager loading or a digest computation, but not across reloads. Fiber[:__vc_path_parser] || ActionView::Resolver::PathParser.new end diff --git a/test/sandbox/test/base_test.rb b/test/sandbox/test/base_test.rb index 4dc4c0ecf..c1f802541 100644 --- a/test/sandbox/test/base_test.rb +++ b/test/sandbox/test/base_test.rb @@ -218,6 +218,35 @@ def test_source_file_contents_are_discarded_after_an_error end end + def test_nested_file_caches_preserve_the_outer_cache + Dir.mktmpdir do |directory| + path = File.join(directory, "temporary_component.html.erb") + File.write(path, "original") + + ViewComponent::CompileCache.with_sidecar_files_cache do + parser = ViewComponent::CompileCache.path_parser + assert_equal "original", ViewComponent::CompileCache.read_file(path) + + ViewComponent::CompileCache.with_sidecar_files_cache do + assert_same parser, ViewComponent::CompileCache.path_parser + File.write(path, "edited") + assert_equal "original", ViewComponent::CompileCache.read_file(path) + end + + assert_same parser, ViewComponent::CompileCache.path_parser + assert_equal "original", ViewComponent::CompileCache.read_file(path) + + assert_raises(RuntimeError) do + ViewComponent::CompileCache.with_sidecar_files_cache { raise "nested failure" } + end + assert_same parser, ViewComponent::CompileCache.path_parser + assert_equal "original", ViewComponent::CompileCache.read_file(path) + end + + assert_equal "edited", ViewComponent::CompileCache.read_file(path) + end + end + def test_template_path_parser_is_shared_only_while_eager_loading parser = nil ViewComponent::CompileCache.with_sidecar_files_cache do diff --git a/test/sandbox/test/cacheable_test.rb b/test/sandbox/test/cacheable_test.rb index d68bbc4f8..dcc0d56b6 100644 --- a/test/sandbox/test/cacheable_test.rb +++ b/test/sandbox/test/cacheable_test.rb @@ -472,6 +472,62 @@ def test_digest_builds_each_component_once clear_digest_cache end + def test_cold_digest_shares_sidecar_directory_listings + clear_digest_cache + directory = File.dirname(CacheableDiamondComponent.identifier) + scans = 0 + glob = Dir.method(:glob) + scan = lambda do |pattern, *args| + scans += 1 if pattern == "#{directory}/*" + glob.call(pattern, *args) + end + + Dir.stub(:glob, scan) { CacheableDiamondComponent.cache_digest } + + assert_equal 1, scans + ensure + clear_digest_cache + end + + def test_file_caching_preserves_the_digest + clear_digest_cache + uncached = ->(&block) { block.call } + without_memoization = ->(_key, &block) { block.call } + expected = ViewComponent::CompileCache.stub(:with_sidecar_files_cache, uncached) do + ViewComponent::CacheDigest.stub(:memoize, without_memoization) do + CacheableDiamondComponent.cache_digest + end + end + clear_digest_cache + + assert_equal expected, CacheableDiamondComponent.cache_digest + ensure + clear_digest_cache + end + + def test_shared_source_files_are_hashed_once_per_computation + resolver = ViewComponent::CacheDigest::Resolver.instance + path = CacheableComponent.identifier + reads = 0 + binread = File.method(:binread) + read = lambda do |filename, *args| + reads += 1 if filename == path + binread.call(filename, *args) + end + expected = resolver.source_for(CacheableComponent) + + File.stub(:binread, read) do + ViewComponent::CacheDigest.memoizing do + assert_equal expected, resolver.source_for(CacheableComponent) + resolver.source_for(CacheableSubclassComponent) + assert_equal 1, reads + end + + assert_equal expected, resolver.source_for(CacheableComponent) + assert_equal 2, reads + end + end + # The children of a dependency whose digest is cached are left out of the # tree, so an expiry while the tree is built has to start the digest over. def test_digest_recomputes_when_digests_expire_while_the_tree_is_built