Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only de-dup/cache FileSystemResolvers in ViewPaths #47786

Merged
merged 3 commits into from Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions actionview/lib/action_view/cache_expiry.rb
Expand Up @@ -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?
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/lookup_context.rb
Expand Up @@ -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
Expand Down
35 changes: 24 additions & 11 deletions actionview/lib/action_view/path_registry.rb
Expand Up @@ -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)
Expand All @@ -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
Expand Down
9 changes: 7 additions & 2 deletions actionview/lib/action_view/path_set.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/rendering.rb
Expand Up @@ -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)

Expand Down
17 changes: 12 additions & 5 deletions actionview/lib/action_view/view_paths.rb
Expand Up @@ -28,14 +28,21 @@ def _prefixes # :nodoc:
end
end

def _build_view_paths(paths) # :nodoc:
return paths if ActionView::PathSet === paths
jhawthorn marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
3 changes: 1 addition & 2 deletions guides/rails_guides/generator.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down