From 4db7ae52f8312ddee193d9902489a6c795c82d51 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 4 Aug 2021 07:04:22 -0700 Subject: [PATCH] Remove cache from Template::Resolver --- .../lib/action_view/template/resolver.rb | 84 +------------------ .../test/actionpack/abstract/layouts_test.rb | 19 ----- .../test/template/resolver_cache_test.rb | 10 --- 3 files changed, 1 insertion(+), 112 deletions(-) delete mode 100644 actionview/test/template/resolver_cache_test.rb diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index e2f3ad363d..b45e38772e 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -49,84 +49,18 @@ module ActionView end end - # Threadsafe template cache - class Cache # :nodoc: - class SmallCache < Concurrent::Map - def initialize(options = {}) - super(options.merge(initial_capacity: 2)) - end - end - - # Preallocate all the default blocks for performance/memory consumption reasons - PARTIAL_BLOCK = lambda { |cache, partial| cache[partial] = SmallCache.new } - PREFIX_BLOCK = lambda { |cache, prefix| cache[prefix] = SmallCache.new(&PARTIAL_BLOCK) } - NAME_BLOCK = lambda { |cache, name| cache[name] = SmallCache.new(&PREFIX_BLOCK) } - KEY_BLOCK = lambda { |cache, key| cache[key] = SmallCache.new(&NAME_BLOCK) } - - # Usually a majority of template look ups return nothing, use this canonical preallocated array to save memory - NO_TEMPLATES = [].freeze - - def initialize - @data = SmallCache.new(&KEY_BLOCK) - end - - def inspect - "#{to_s[0..-2]} keys=#{@data.size}>" - end - - # Cache the templates returned by the block - def cache(key, name, prefix, partial, locals) - @data[key][name][prefix][partial][locals] ||= canonical_no_templates(yield) - end - - def clear - @data.clear - end - - # Get the cache size. Do not call this - # method. This method is not guaranteed to be here ever. - def size # :nodoc: - size = 0 - @data.each_value do |v1| - v1.each_value do |v2| - v2.each_value do |v3| - v3.each_value do |v4| - size += v4.size - end - end - end - end - - size - end - - private - def canonical_no_templates(templates) - templates.empty? ? NO_TEMPLATES : templates - end - end - cattr_accessor :caching, default: true class << self alias :caching? :caching end - def initialize - @cache = Cache.new - end - def clear_cache - @cache.clear end # Normalizes the arguments and passes it on to find_templates. def find_all(name, prefix = nil, partial = false, details = {}, key = nil, locals = []) - locals = locals.map(&:to_s).sort!.freeze - - cached(key, [name, prefix, partial], details, locals) do - _find_all(name, prefix, partial, details, key, locals) - end + _find_all(name, prefix, partial, details, key, locals) end def all_template_paths # :nodoc: @@ -147,22 +81,6 @@ module ActionView def find_templates(name, prefix, partial, details, locals = []) raise NotImplementedError, "Subclasses must implement a find_templates(name, prefix, partial, details, locals = []) method" end - - # Handles templates caching. If a key is given and caching is on - # always check the cache before hitting the resolver. Otherwise, - # it always hits the resolver but if the key is present, check if the - # resolver is fresher before returning it. - def cached(key, path_info, details, locals) - name, prefix, partial = path_info - - if key - @cache.cache(key, name, prefix, partial, locals) do - yield - end - else - yield - end - end end # A resolver that loads files from the filesystem. diff --git a/actionview/test/actionpack/abstract/layouts_test.rb b/actionview/test/actionpack/abstract/layouts_test.rb index 15643c161d..0e3ceb53ce 100644 --- a/actionview/test/actionpack/abstract/layouts_test.rb +++ b/actionview/test/actionpack/abstract/layouts_test.rb @@ -285,25 +285,6 @@ module AbstractControllerTests assert_equal "With String hello less than 3 bar", controller.response_body end - test "cache should not grow when locals change for a string template" do - cache = WithString.view_paths.paths.first.instance_variable_get(:@cache) - - controller = WithString.new - controller.process(:index) # heat the cache - - size = cache.size - - 10.times do |x| - controller = WithString.new - controller.define_singleton_method :index do - render template: ActionView::Template::Text.new("Hello string!"), locals: { "x#{x}": :omg } - end - controller.process(:index) - end - - assert_equal size, cache.size - end - test "when layout is specified as a string, render with that layout" do controller = WithString.new controller.process(:index) diff --git a/actionview/test/template/resolver_cache_test.rb b/actionview/test/template/resolver_cache_test.rb deleted file mode 100644 index 625bf653dd..0000000000 --- a/actionview/test/template/resolver_cache_test.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -require "abstract_unit" - -class ResolverCacheTest < ActiveSupport::TestCase - def test_inspect_shields_cache_internals - ActionView::LookupContext::DetailsKey.clear - assert_match %r(#>), ActionView::Resolver.new.inspect - end -end