Should GC.start be run before reporting results?

[dblock]

Coming from https://github.com/michaelherold/benchmark-memory/issues/9. Output was confusing without GC.start.

[michaelherold]

Good question, @dblock. I think that makes sense to me. Apologies for the delay in response!

[AlexWayfer]

I can try to make a PR, if you want.

[dblock]

Don't let me stop by @AlexWayfer :)

[AlexWayfer]

I'm interesting in fresh dependencies, so I want to wait for #11 and #10.

[AlexWayfer]

Now #14 (and merged #16 with #17 would be good too).

[michaelherold]

Alright, digging in here. If I am understanding the intent of the benchmark, this is a clearer way to write it:

Benchmark code

require 'set'
require 'benchmark/memory'

class Retainer
  def initialize(container)
    @container = container
  end

  def retain!
    @container << { x: 1 }
  end
end

ra = Retainer.new([])
rs = Retainer.new(Set.new)

Benchmark.memory do |b|
  calls = 10_001

  b.report('using Array') do
    calls.times { ra.retain! }
  end

  b.report('using Set') do
    calls.times { rs.retain! }
  end

  b.compare!
end

Running this gives the following output:

Benchmark output

Calculating -------------------------------------
         using Array     2.320M memsize (     2.320M retained)
                        10.001k objects (    10.001k retained)
                         0.000  strings (     0.000  retained)
           using Set     2.321M memsize (   928.000  retained)
                        10.004k objects (     4.000  retained)
                         0.000  strings (     0.000  retained)

Comparison:
         using Array:    2320232 allocated
           using Set:    2320928 allocated - 1.00x more

If I'm understanding the source of the question, you were finding this confusing because the report is about the magnitude of allocations rather than the magnitude of retentions. In that case, I don't think that calling GC.start would do what you expect because it's already done as part of the measurement.

I think what I would like to do to address this is twofold:

1. Output a comparison of retentions as well as allocations
2. Allow you to use allocations OR retentions to sort the comparison

(1) would make the output less confusing by default and (2) would allow you to tune the tool to be better suited for testing memory leaks.

What do you think? Would those two changes have made the situation less confusing?

[AlexWayfer]

I'm reviewing the original report and a bit confused:

Yeah, @setup << { x: 1 } with @setup as a Set will prevent duplicates inside itself, but there are { x: 1 } initializations anyway, and they're all in a report, so… objects created anyway. And now, while I'm writing it, I see 1.680M retained vs 168.000 retained, and yes, having their comparison under Comparison: "header" can make things more understandable. Also maybe renaming or a hint like "retained after garbage collection".