Skip to content

Commit 597b56c

Browse files
Optimize TagBuilder tag generation
Currently there's about a 35% difference between tags generated using the `TagBuilder` and tags generated by passing a positional argument to `#tag`. This commit optimizes `TagBuilder` to reduce that difference down to 13%. The first change is to perform less hash allocations by not splatting the options twice in the `TagBuilder` (one at the `tag.a` invocation, and one at `tag_string`). The extra splat for `tag_string` was moved into `method_missing` since that is the only other caller of this private method. The other change is to only escape the content in `tag_string` if it a non-empty. Additionally, a test was tweaked to ensure that passing `options` to a `self_closing_element` is tested as it was previously not. Benchmark: ``` require "action_view" require "benchmark/ips" class Foo include ActionView::Helpers end helpers = Foo.new Benchmark.ips do |x| x.report("tag") { helpers.tag("a", href: "foo") } x.report("tag_builder") { helpers.tag.a(href: "foo") } x.compare! end ``` Before: ``` ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22] Warming up -------------------------------------- tag 67.180k i/100ms tag_builder 50.267k i/100ms Calculating ------------------------------------- tag 673.064k (± 0.4%) i/s - 3.426M in 5.090520s tag_builder 504.971k (± 0.4%) i/s - 2.564M in 5.076842s Comparison: tag: 673063.7 i/s tag_builder: 504971.4 i/s - 1.33x slower ``` After: ``` ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22] Warming up -------------------------------------- tag 67.374k i/100ms tag_builder 59.702k i/100ms Calculating ------------------------------------- tag 670.837k (± 0.4%) i/s - 3.369M in 5.021714s tag_builder 592.727k (± 1.3%) i/s - 2.985M in 5.037088s Comparison: tag: 670836.6 i/s tag_builder: 592726.7 i/s - 1.13x slower ``` Co-authored-by: Sean Doyle <[email protected]>
1 parent 299900f commit 597b56c

File tree

2 files changed

+8
-8
lines changed

2 files changed

+8
-8
lines changed

actionview/lib/action_view/helpers/tag_helper.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ def self.define_element(name, code_generator:, method_name: name.to_s.underscore
5252
code_generator.define_cached_method(method_name, namespace: :tag_builder) do |batch|
5353
batch.push(<<~RUBY) unless instance_methods.include?(method_name.to_sym)
5454
def #{method_name}(content = nil, escape: true, **options, &block)
55-
tag_string(#{name.inspect}, content, escape: escape, **options, &block)
55+
tag_string("#{name}", content, options, escape: escape, &block)
5656
end
5757
RUBY
5858
end
@@ -70,7 +70,7 @@ def #{method_name}(content = nil, escape: true, **options, &block)
7070
positional argument will raise, and using a block will have
7171
no effect.
7272
TEXT
73-
tag_string("#{name}", content, escape: escape, **options, &block)
73+
tag_string("#{name}", content, options, escape: escape, &block)
7474
else
7575
self_closing_tag_string("#{name}", options, escape, ">")
7676
end
@@ -84,7 +84,7 @@ def self.define_self_closing_element(name, code_generator:, method_name: name.to
8484
batch.push(<<~RUBY)
8585
def #{method_name}(content = nil, escape: true, **options, &block)
8686
if content || block
87-
tag_string("#{name}", content, escape: escape, **options, &block)
87+
tag_string("#{name}", content, options, escape: escape, &block)
8888
else
8989
self_closing_tag_string("#{name}", options, escape)
9090
end
@@ -239,7 +239,7 @@ def attributes(attributes)
239239
tag_options(attributes.to_h).to_s.strip.html_safe
240240
end
241241

242-
def tag_string(name, content = nil, escape: true, **options, &block)
242+
def tag_string(name, content = nil, options, escape: true, &block)
243243
content = @view_context.capture(self, &block) if block
244244

245245
content_tag_string(name, content, options, escape)
@@ -252,7 +252,7 @@ def self_closing_tag_string(name, options, escape = true, tag_suffix = " />")
252252
def content_tag_string(name, content, options, escape = true)
253253
tag_options = tag_options(options, escape) if options
254254

255-
if escape
255+
if escape && content.present?
256256
content = ERB::Util.unwrapped_html_escape(content)
257257
end
258258
"<#{name}#{tag_options}>#{PRE_CONTENT_STRINGS[name]}#{content}</#{name}>".html_safe
@@ -334,12 +334,12 @@ def respond_to_missing?(*args)
334334
true
335335
end
336336

337-
def method_missing(called, ...)
337+
def method_missing(called, *args, escape: true, **options, &block)
338338
name = called.name.dasherize
339339

340340
TagHelper.ensure_valid_html5_tag_name(name)
341341

342-
tag_string(name, ...)
342+
tag_string(name, *args, options, escape: escape, &block)
343343
end
344344
end
345345

actionview/test/template/tag_helper_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def test_tag_builder_self_closing_tag
4545
end
4646

4747
def test_tag_builder_self_closing_tag_with_content
48-
assert_equal "<svg><circle><desc>A circle</desc></circle></svg>", tag.svg { tag.circle { tag.desc "A circle" } }
48+
assert_equal "<svg><circle r=\"5\"><desc>A circle</desc></circle></svg>", tag.svg { tag.circle(r: "5") { tag.desc "A circle" } }
4949
end
5050

5151
def test_tag_builder_defines_methods_to_build_html_elements

0 commit comments

Comments
 (0)