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

Optimize TagBuilder tag generation #50737

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

skipkayhil
Copy link
Member

@skipkayhil skipkayhil commented Jan 12, 2024

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.

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

@skipkayhil skipkayhil force-pushed the hm-further-optimize-tag-builder branch 3 times, most recently from 2d87fd2 to 13b64a0 Compare January 12, 2024 22:01
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 <seanpdoyle@users.noreply.github.com>
@skipkayhil skipkayhil force-pushed the hm-further-optimize-tag-builder branch from 13b64a0 to 597b56c Compare January 16, 2024 22:52
@byroot byroot merged commit 4816684 into rails:main Jan 18, 2024
4 checks passed
@skipkayhil skipkayhil deleted the hm-further-optimize-tag-builder branch January 18, 2024 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants