diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index dfac93bd3..1922e6a0e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Reduce source-file reads and dependency-scanning allocations during cold cache digests and boot-time precomputation. + + *Joel Hawksley* + * 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* diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index 3700c91a8..ba6752b6b 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -368,7 +368,8 @@ def partition_render_calls(program, nesting = []) each_node(program, nesting) do |node, scope| if node.is_a?(Prism::ConstantReadNode) || node.is_a?(Prism::ConstantPathNode) - names << [node.slice, scope] if CONSTANT_NAME.match?(node.slice) + name = node.slice + names << [name, scope] if CONSTANT_NAME.match?(name) elsif render_call?(node) if rendered_component(node, scope) component_calls << node @@ -389,9 +390,10 @@ def render_call?(node) # Class/module bodies add one lexical scope, even for qualified names: # `class UI::Card` adds Card, not UI, to Module.nesting. def each_node(program, nesting = []) - nodes = [[program, nesting]] - while (entry = nodes.pop) - node, scope = entry + # Keep scopes and nodes in one stack rather than allocating a pair per node. + nodes = [nesting, program] + while (node = nodes.pop) + scope = nodes.pop yield node, scope body = nil @@ -403,7 +405,7 @@ def each_node(program, nesting = []) end node.compact_child_nodes.reverse_each do |child| - nodes << [child, child.equal?(body) ? body_scope : scope] + nodes.push(child.equal?(body) ? body_scope : scope, child) end end end diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index 5b03b8bb2..d8654936e 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -245,7 +245,16 @@ def ruby_sources(component) end def file_digest(path) - CacheDigest.memoize([:file_digest, path]) { ActiveSupport::Digest.hexdigest(::File.binread(path)) } + CacheDigest.memoize([:file_digest, path]) do + # Reuse source reads unless text mode can transcode or translate newlines. + source = if ::File::ALT_SEPARATOR.nil? && + (Encoding.default_internal.nil? || Encoding.default_internal == Encoding.default_external) + CompileCache.read_file(path) + else + ::File.binread(path) + end + ActiveSupport::Digest.hexdigest(source) + end end # Built once at load time rather than memoized, so no class-level state diff --git a/test/sandbox/test/cacheable_test.rb b/test/sandbox/test/cacheable_test.rb index dcc0d56b6..0fd9b23d6 100644 --- a/test/sandbox/test/cacheable_test.rb +++ b/test/sandbox/test/cacheable_test.rb @@ -175,6 +175,26 @@ def test_absolute_dependencies_ignore_lexical_nesting assert_equal ["view_component/cache_digest/child_component"], dependencies end + def test_dependency_discovery_restores_lexical_scope_after_nested_bodies + source = <<~RUBY + module CacheDigestNamespace + class ParentComponent + ChildComponent + end + ChildComponent + end + ChildComponent + RUBY + dependencies, partials = ViewComponent::CacheDigest.renders_in(source, "test/template") + + assert_equal [ + "view_component/cache_digest/cache_digest_namespace/parent_component", + "view_component/cache_digest/cache_digest_namespace/child_component", + "view_component/cache_digest/child_component" + ], dependencies + assert_empty partials + end + def test_qualified_class_declarations_do_not_add_enclosing_namespaces_to_nesting source = "class CacheDigestNamespace::ParentComponent; render ChildComponent.new; end" dependencies, = ViewComponent::CacheDigest.renders_in(source, "test/template") @@ -507,24 +527,77 @@ def test_file_caching_preserves_the_digest 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) + source = File.binread(CacheableComponent.identifier) + hashes = 0 + hexdigest = ActiveSupport::Digest.method(:hexdigest) + hash_source = lambda do |content| + hashes += 1 if content.b == source + hexdigest.call(content) end expected = resolver.source_for(CacheableComponent) - File.stub(:binread, read) do + ActiveSupport::Digest.stub(:hexdigest, hash_source) do ViewComponent::CacheDigest.memoizing do assert_equal expected, resolver.source_for(CacheableComponent) resolver.source_for(CacheableSubclassComponent) - assert_equal 1, reads + assert_equal 1, hashes end assert_equal expected, resolver.source_for(CacheableComponent) - assert_equal 2, reads + assert_equal 2, hashes + end + end + + def test_cold_digests_share_ruby_reads_between_hashing_and_dependency_discovery + skip "Text reads can change source bytes" if File::ALT_SEPARATOR || + (Encoding.default_internal && Encoding.default_internal != Encoding.default_external) + + clear_digest_cache + path = CacheableComponent.identifier + reads = 0 + read = File.method(:read) + binread = File.method(:binread) + + File.stub(:read, lambda { |filename, *args| + reads += 1 if filename == path + read.call(filename, *args) + }) do + File.stub(:binread, lambda { |filename, *args| + reads += 1 if filename == path + binread.call(filename, *args) + }) do + CacheableComponent.cache_digest + assert_equal 1, reads + + clear_digest_cache + CacheableComponent.cache_digest + assert_equal 2, reads + end + end + ensure + clear_digest_cache + end + + def test_source_hashes_use_binary_reads_when_text_reads_transcode + path = CacheableComponent.identifier + expected = ActiveSupport::Digest.hexdigest(File.binread(path)) + + Encoding.stub(:default_internal, Encoding::UTF_16LE) do + File.stub(:read, ->(*) { flunk "read transcoded source" }) do + assert_equal expected, ViewComponent::CacheDigest::Resolver.instance.send(:file_digest, path) + end + end + end + + def test_shared_source_reads_preserve_encoded_bytes_and_line_endings + Dir.mktmpdir do |directory| + path = File.join(directory, "source.rb") + File.binwrite(path, "# caf\u00e9\r\n") + expected = ActiveSupport::Digest.hexdigest(File.binread(path)) + + ViewComponent::CompileCache.with_sidecar_files_cache do + assert_equal expected, ViewComponent::CacheDigest::Resolver.instance.send(:file_digest, path) + end end end