Avoid allocating new string when output_postamble is blank

[mitchellhenke]

What are you trying to accomplish?

This pull request is strictly to improve performance by reducing allocations

What approach did you choose and why?

The change is exclusively focused on the output_postamble method and the default empty string. The concatenation calls https://github.com/rails/rails/blob/v7.0.8/activesupport/lib/active_support/core_ext/string/output_safety.rb#L247-249 which includes a duplication of the original string and concatenation of an empty one.

Avoiding the concatenation saves an allocation of the rendered template, which can be quite large.

This was found while working on #1884.

Anything you want to highlight for special attention from reviewers?

The code changes are quite small, but a potentially significant thing I'm not sure of is whether there is a good way to add tests.

[mitchellhenke]

CI is failing, but I'm not sure how to address it. I'm assuming it has something to do with changes on Rails main branch rather than any changes here.

I ran a slightly modified version of the existing benchmarks (Ruby 3.2.2 on an Apple M1 Pro), and it does show a pretty good performance improvement when there is no output_postamble (branch here):

$ bundle exec rake partial_benchmark

Warming up --------------------------------------
   component no skip   511.000  i/100ms
      component skip   610.000  i/100ms
Calculating -------------------------------------
   component no skip      5.053k (± 4.4%) i/s -     50.589k in  10.037196s
      component skip      6.110k (± 1.7%) i/s -     61.610k in  10.087141s

Comparison:
      component skip:     6109.6 i/s
>  component no skip:     5052.5 i/s - 1.21x  slower

Warming up --------------------------------------
      inline no skip   594.000  i/100ms
         inline skip   737.000  i/100ms
Calculating -------------------------------------
      inline no skip      5.911k (± 1.0%) i/s -     59.400k in  10.050332s
         inline skip      7.306k (± 0.7%) i/s -     73.700k in  10.088450s

Comparison:
         inline skip:     7305.8 i/s
>     inline no skip:     5910.9 i/s - 1.24x  slower

Warming up --------------------------------------
     partial no skip    98.000  i/100ms
        partial skip    92.000  i/100ms
Calculating -------------------------------------
     partial no skip    906.370  (± 4.2%) i/s -      9.114k in  10.074217s
        partial skip    939.161  (± 4.9%) i/s -      9.384k in  10.016932s

Comparison:
        partial skip:      939.2 i/s
>    partial no skip:      906.4 i/s - same-ish: difference falls within error

The benchmarks in CI show improvements as well:
This PR

Calculating -------------------------------------
           component      3.066k (± 1.5%) i/s -     30.800k in  10.046625s
              inline      3.673k (± 1.5%) i/s -     37.067k in  10.093854s
             partial    423.461  (± 2.1%) i/s -      4.264k in  10.073852s

Comparison:
              inline:     3673.2 i/s
           component:     3066.5 i/s - 1.20x  slower
             partial:      423.5 i/s - 8.67x  slower

main

Calculating -------------------------------------
           component      2.562k (± 1.0%) i/s -     25.856k in  10.091217s
              inline      2.925k (± 0.9%) i/s -     29.300k in  10.019037s
             partial    418.358  (± 1.0%) i/s -      4.223k in  10.095338s

Comparison:
              inline:     2924.7 i/s
           component:     2562.5 i/s - 1.14x  slower
             partial:      418.4 i/s - 6.99x  slower

[reeganviljoen]

@mitchellhenke sorry about the late reply, I can't seem to find your issue, @joelhawksley are you able to help

[mitchellhenke]

@reeganviljoen apologies, I didn't create an issue for this one, but I am happy to if it is helpful

[mitchellhenke]

Thank you for the review and merge!