Move object allocation out of loop

[schneems]

Right now BenchmarkCleaner allocates hundreds of strings on every request with an exception. This patch moves those strings to be generated at boot once and re-used.

Bench Methods

I took a modified form of CodeTriage https://github.com/schneems/codetriage-ko1-test-app/blob/master/perf.rake and ran given rake tasks with and without the patch to BacktraceCleaner.

I made an endpoint results in exception

  def index
    raise “foo"
  end

The gem memory_profiler was used to capture objects allocated for a single request. Then benchmark/ips was used to test the speed of the patch.

You will see that we use less objects and the code becomes measurably faster with this patch.

With patch:

Memory for one request:

Total allocated 7441
Total retained 37

Requests per second:

Calculating -------------------------------------
                 ips         4 i/100ms
-------------------------------------------------
                 ips       43.0 (±4.7%) i/s -        216 in   5.037733s

Without patch:

Memory used for one request:

Total allocated 11599
Total retained 35 

Requests per second:

Calculating -------------------------------------
                 ips         3 i/100ms
-------------------------------------------------
                 ips       39.4 (±7.6%) i/s -        198 in   5.052783s

Analysis

The patch is faster:

(43.0 - 39.4 ) / 39.4 * 100
# => 9 # % ~ speed bump

It also allocates less objects:

11599 - 7441
# => 4158

These strings are allocated on EVERY SINGLE REQUEST. This patch saves us 4158 objects PER REQUEST with exception.

Faster errors == Faster applications

[schneems]

They probably should be 🍦.freeze

[jeremy]

👍 noticed this too

[matthewd]

If those blocks are hit often enough to matter, perhaps we should look to avoid the sub allocations too?

[schneems]

Not entirely sure why but the sub seems to have much less impact than the completely new string allocation

Here's the sub

require 'benchmark/ips'

str1 = "foo"
str2 = "foo"

regex = /f/
empty = ''

Benchmark.ips do |x|
  x.report("sub")  { str1.dup.sub(regex, empty) }
  x.report("sub!") { str2.dup.sub!(regex, empty) }
end

The results show the in place sub being faster, but not by much

Calculating -------------------------------------
                 sub     56957 i/100ms
                sub!     60843 i/100ms
-------------------------------------------------
                 sub  1071631.7 (±5.4%) i/s -    5353958 in   5.013054s
                sub!  1184414.5 (±7.0%) i/s -    5901771 in   5.012682s

Comparing creating a string to using an existing string:

Benchmark.ips do |x|
  x.report("new string")  { '' }
  x.report("previously allocated string") { empty }
end

It is much slower

Calculating -------------------------------------
          new string    110226 i/100ms
previously allocated string
                        111156 i/100ms
-------------------------------------------------
          new string  5018702.8 (±10.3%) i/s -   24800850 in   5.010430s
previously allocated string
                      8538568.3 (±8.0%) i/s -   42350436 in   5.003677s

I think it would be unexpected for a cleaner to modify the strings in place, though maybe it's fine. I'm not too familiar with all the use cases here. If someone else wants it and thinks its worth it it, let me know. I think changing sub patch would be better as a separate PR if there are problems and it needs to be reverted. I'm pretty solid on this optimization as is.