Visitar URL original
Reduce allocation overhead of #50623 by joelhawksley · Pull Request #1 · joelhawksley/rails · GitHub
Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions actionview/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
* Reduce allocation overhead of rendering `render_in`-compatible objects.

PR #50623 introduced a ~5x allocation regression for objects rendered via
`render(renderable)` (including ViewComponent) because every such call
began flowing through the standard template rendering pipeline. This
change memoizes per-class metadata used by `Template::Renderable`,
avoids redundant `prepend_formats` writes, reuses the `TemplateRenderer`
within a `Renderer`, gates the `render_template.action_view`
notification payload construction on active subscribers, and reuses
a pooled `Template::Renderable` when no block is supplied.

*Joel Hawksley*

* Allow `translate`'s (and `t`'s) `scope:` option to be resolved relative to
the current template when it starts with a period, mirroring the existing
behavior for the key argument.
Expand Down
5 changes: 4 additions & 1 deletion actionview/lib/action_view/renderer/abstract_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ def prepend_formats(formats) # :doc:
formats = Array(formats)
return if formats.empty? || @lookup_context.html_fallback_for_js

@lookup_context.formats = formats | @lookup_context.formats
current = @lookup_context.formats
return if (formats - current).empty?

@lookup_context.formats = formats | current
end

def build_rendered_template(content, template)
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/renderer/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def cache_hits # :nodoc:

private
def render_template_to_object(context, options, &block)
TemplateRenderer.new(@lookup_context).render(context, options, &block)
(@_template_renderer ||= TemplateRenderer.new(@lookup_context)).render(context, options, &block)
end

def render_partial_to_object(context, options, &block)
Expand Down
28 changes: 21 additions & 7 deletions actionview/lib/action_view/renderer/template_renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

module ActionView
class TemplateRenderer < AbstractRenderer # :nodoc:
RENDER_TEMPLATE_NOTIFICATION = "render_template.action_view"
private_constant :RENDER_TEMPLATE_NOTIFICATION

def render(context, options, &block)
@details = extract_details(options)
template = determine_template(options, &block)
Expand Down Expand Up @@ -41,7 +44,14 @@ def determine_template(options, &block)
end
Template::Inline.new(options[:inline], "inline template", handler, locals: keys, format: format)
elsif options.key?(:renderable)
Template::Renderable.new(options[:renderable], &block)
if block.nil?
pooled = (@_pooled_renderable ||= Template::Renderable.allocate)
pooled.instance_variable_set(:@renderable, options[:renderable])
pooled.instance_variable_set(:@block, nil)
pooled
else
Template::Renderable.new(options[:renderable], &block)
end
elsif options.key?(:template)
if options[:template].respond_to?(:render)
options[:template]
Expand All @@ -57,12 +67,16 @@ def determine_template(options, &block)
# supplied as well.
def render_template(view, template, layout_name, locals)
render_with_layout(view, template, layout_name, locals) do |layout|
ActiveSupport::Notifications.instrument(
"render_template.action_view",
identifier: template.identifier,
layout: layout && layout.virtual_path,
locals: locals
) do
if ActiveSupport::Notifications.notifier.listening?(RENDER_TEMPLATE_NOTIFICATION)
ActiveSupport::Notifications.instrument(
RENDER_TEMPLATE_NOTIFICATION,
identifier: template.identifier,
layout: layout && layout.virtual_path,
locals: locals
) do
template.render(view, locals) { |*name| view._layout_for(*name) }
end
else
template.render(view, locals) { |*name| view._layout_for(*name) }
end
end
Expand Down
25 changes: 22 additions & 3 deletions actionview/lib/action_view/template/renderable.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
# frozen_string_literal: true

require "concurrent/map"

module ActionView
class Template
# = Action View Renderable Template for objects that respond to #render_in
class Renderable # :nodoc:
RENDER_IN_ARITY = Concurrent::Map.new
FORMAT_RESPONDER = Concurrent::Map.new
private_constant :RENDER_IN_ARITY, :FORMAT_RESPONDER

def initialize(renderable, &block)
@renderable = renderable
@block = block
Expand All @@ -14,9 +20,16 @@ def identifier
end

def render(context, locals)
render_in_method = Kernel.instance_method(:method).bind_call(@renderable, :render_in)
klass = @renderable.class
arity = if klass.method_defined?(:render_in)
RENDER_IN_ARITY.fetch_or_store(klass) do
klass.instance_method(:render_in).arity
end
else
Kernel.instance_method(:method).bind_call(@renderable, :render_in).arity
end

if render_in_method.arity == 1
if arity == 1
ActionView.deprecator.warn <<~WARN
Action View support for #render_in without options is deprecated.

Expand All @@ -36,7 +49,13 @@ def render(context, locals)
end

def format
@renderable.try(:format)
klass = @renderable.class
responds = if klass.method_defined?(:format)
FORMAT_RESPONDER.fetch_or_store(klass) { true }
else
@renderable.respond_to?(:format)
end
responds ? @renderable.format : nil
end
end
end
Expand Down
Loading