Force a major GC and compact before measuring RSS#481
Conversation
This will hopefully remove any variance caused by one benchmark run having just hit a GC and one not.
|
Aren't all benchmarks each run in their own process so this shouldn't matter, at least the part about a benchmark affecting another? |
| # Full GC then compact before measuring RSS so fragmentation doesn't inflate the number. | ||
| GC.start(full_mark: true, immediate_sweep: true) | ||
| GC.compact if GC.respond_to?(:compact) |
There was a problem hiding this comment.
I think this is a non-trivial overhead, maybe we should just report maxrss which doesn't need this.
In the worst case the extra GC might even cause more memory usage potentially.
GC.compact is also somewhat brittle as it can break if a single extension doesn't do the right thing, so it feels rather dangerous to do.
There was a problem hiding this comment.
I disagree that GC.compact makes this brittle. An extension that reports compaction support and breaks compaction is a correctness issue, and we shouldn't be benchmarking against broken code.
I am amenable to reporting maxrss, but that fundamentally changes the semantics of the benchmark results so I think that is a change we'd have to be more careful about. Or at least communicate effectively on rubybench.github.io or speed.ruby-lang.org.
There was a problem hiding this comment.
An extension that reports compaction support and breaks compaction is a correctness issue, and we shouldn't be benchmarking against broken code.
It can be more subtle than that, e.g. holding to a Ruby object in a VALUE struct field or global variable without marking it and also holding it from a Ruby object directly works without GC.compact, but breaks with GC.compact.
Such an extension would not report compaction support, but still break.
|
Did you notice any improvement in practice with this change? |
Anecdotally I have noticed smaller variance in RSS results when benchmarking my current branch repeatedly on an AWS bare metal instance set up specifically to benchmark. I haven't prepared any concrete documentation yet though. I propose that we let it run for a few days and measure the effects on the memory benchmarks in https://github.com/rubybench/rubybench-data/ before deciding to revert. This is minimal risk as if we do decide to revert, then the data from our testing date range can be easily backfilled. |
|
I'd expect this will also change the reported RSS value a lot, before it was meant to be measured right after the last iteration but now with the GC before it measures "which objects are globally reachable" which should be much smaller and probably not interesting as it would be far off from the actual usage during benchmarking. |
3 participants
This will hopefully remove any variance caused by one benchmark run having just hit a GC and one not.