Merge pull request #47786 from jhawthorn/view_caching_attempt4

Only de-dup/cache FileSystemResolvers in ViewPaths

actionview/lib/action_view/cache_expiry.rb

@@ -12,10 +12,7 @@ def initialize(watcher:, &block)
 
         rebuild_watcher
 
-        _self = self
-        ActionView::PathRegistry.singleton_class.set_callback(:build_file_system_resolver, :after) do
-          _self.send(:rebuild_watcher)
-        end
+        ActionView::PathRegistry.file_system_resolver_hooks << method(:rebuild_watcher)
       end
 
       def updated?

actionview/lib/action_view/lookup_context.rb

@@ -83,9 +83,9 @@ def self.digest_caches
         @digest_cache.values
       end
 
-      def self.view_context_class(klass)
+      def self.view_context_class
         @view_context_mutex.synchronize do
-          @view_context_class ||= klass.with_empty_template_cache
+          @view_context_class ||= ActionView::Base.with_empty_template_cache
         end
       end
     end

actionview/lib/action_view/path_registry.rb

@@ -3,11 +3,12 @@
 module ActionView # :nodoc:
   module PathRegistry # :nodoc:
     @view_paths_by_class = {}
-    @file_system_resolvers = Concurrent::Map.new
+    @file_system_resolvers = {}
+    @file_system_resolver_mutex = Mutex.new
+    @file_system_resolver_hooks = []
 
     class << self
-      include ActiveSupport::Callbacks
-      define_callbacks :build_file_system_resolver
+      attr_reader :file_system_resolver_hooks
     end
 
     def self.get_view_paths(klass)
@@ -18,17 +19,29 @@ def self.set_view_paths(klass, paths)
       @view_paths_by_class[klass] = paths
     end
 
-    def self.file_system_resolver(path)
-      path = File.expand_path(path)
-      resolver = @file_system_resolvers[path]
-      unless resolver
-        run_callbacks(:build_file_system_resolver) do
-          resolver = @file_system_resolvers.fetch_or_store(path) do
-            FileSystemResolver.new(path)
+    def self.cast_file_system_resolvers(paths)
+      paths = Array(paths)
+
+      @file_system_resolver_mutex.synchronize do
+        built_resolver = false
+        paths = paths.map do |path|
+          case path
+          when String, Pathname
+            path = File.expand_path(path)
+            @file_system_resolvers[path] ||=
+              begin
+                built_resolver = true
+                FileSystemResolver.new(path)
+              end
+          else
+            path
           end
         end
+
+        file_system_resolver_hooks.each(&:call) if built_resolver
       end
-      resolver
+
+      paths
     end
 
     def self.all_resolvers

actionview/lib/action_view/path_set.rb

@@ -32,7 +32,8 @@ def compact
       PathSet.new paths.compact
     end
 
-    def +(array)
+    def +(other)
+      array = Array === other ? other : other.paths
       PathSet.new(paths + array)
     end
 
@@ -67,7 +68,11 @@ def typecast(paths)
         paths.map do |path|
           case path
           when Pathname, String
-            ActionView::PathRegistry.file_system_resolver(path.to_s)
+            # This path should only be reached by "direct" users of
+            # ActionView::Base (not using the ViewPaths or Renderer modules).
+            # We can't cache/de-dup the file system resolver in this case as we
+            # don't know which compiled_method_container we'll be rendering to.
+            FileSystemResolver.new(path)
           when Resolver
             path
           else

actionview/lib/action_view/rendering.rb

@@ -80,7 +80,7 @@ def eager_load!
       end
 
       def view_context_class
-        klass = ActionView::LookupContext::DetailsKey.view_context_class(ActionView::Base)
+        klass = ActionView::LookupContext::DetailsKey.view_context_class
 
         @view_context_class ||= build_view_context_class(klass, supports_path?, _routes, _helpers)
 

actionview/lib/action_view/view_paths.rb

@@ -28,14 +28,21 @@ def _prefixes # :nodoc:
         end
       end
 
+      def _build_view_paths(paths) # :nodoc:
+        return paths if ActionView::PathSet === paths
+
+        paths = ActionView::PathRegistry.cast_file_system_resolvers(paths)
+        ActionView::PathSet.new(paths)
+      end
+
       # Append a path to the list of view paths for this controller.
       #
       # ==== Parameters
       # * <tt>path</tt> - If a String is provided, it gets converted into
       #   the default view path. You may also provide a custom view path
       #   (see ActionView::PathSet for more information)
       def append_view_path(path)
-        self._view_paths = ActionView::PathSet.new(view_paths.to_a + Array(path))
+        self._view_paths = view_paths + _build_view_paths(path)
       end
 
       # Prepend a path to the list of view paths for this controller.
@@ -45,7 +52,7 @@ def append_view_path(path)
       #   the default view path. You may also provide a custom view path
       #   (see ActionView::PathSet for more information)
       def prepend_view_path(path)
-        self._view_paths = ActionView::PathSet.new(Array(path) + view_paths.to_a)
+        self._view_paths = _build_view_paths(path) + view_paths
       end
 
       # A list of all of the default view paths for this controller.
@@ -59,7 +66,7 @@ def view_paths
       # * <tt>paths</tt> - If a PathSet is provided, use that;
       #   otherwise, process the parameter into a PathSet.
       def view_paths=(paths)
-        self._view_paths = ActionView::PathSet.new(Array(paths))
+        self._view_paths = _build_view_paths(paths)
       end
 
       private
@@ -94,7 +101,7 @@ def details_for_lookup
     #   the default view path. You may also provide a custom view path
     #   (see ActionView::PathSet for more information)
     def append_view_path(path)
-      lookup_context.append_view_paths(Array(path))
+      lookup_context.append_view_paths(self.class._build_view_paths(path))
     end
 
     # Prepend a path to the list of view paths for the current LookupContext.
@@ -104,7 +111,7 @@ def append_view_path(path)
     #   the default view path. You may also provide a custom view path
     #   (see ActionView::PathSet for more information)
     def prepend_view_path(path)
-      lookup_context.prepend_view_paths(Array(path))
+      lookup_context.prepend_view_paths(self.class._build_view_paths(path))
     end
   end
 end

guides/rails_guides/generator.rb

@@ -26,7 +26,6 @@ def initialize(edge:, version:, all:, only:, epub:, language:, direction: nil)
       @epub      = epub
       @language  = language
       @direction = direction || "ltr"
-      @view = ActionView::Base.with_empty_template_cache
 
       if @epub
         register_special_mime_types
@@ -133,7 +132,7 @@ def generate_guide(guide, output_file)
         puts "Generating #{guide} as #{output_file}"
         layout = @epub ? "epub/layout" : "layout"
 
-        view = @view.with_view_paths(
+        view = ActionView::Base.with_empty_template_cache.with_view_paths(
           [@source_dir],
           edge:     @edge,
           version:  @version,