Skip to content

Force a major GC and compact before measuring RSS #481

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.

Sign up for GitHub

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

Merged
eightbitraptor merged 1 commit into from
Feb 18, 2026
Merged

Force a major GC and compact before measuring RSS #481

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion harness/harness-common.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ def return_results(warmup_iterations, bench_iterations)
yjit_stats = RubyVM::YJIT.runtime_stats if defined?(RubyVM::YJIT.enabled?) && RubyVM::YJIT.enabled?
zjit_stats = RubyVM::ZJIT.stats if defined?(RubyVM::ZJIT.enabled?) && RubyVM::ZJIT.enabled?

# Collect our own peak mem usage as soon as reasonable after finishing the last iteration.
# 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)
Comment on lines +149 to +151
Copy link
Member

@eregon eregon Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@eightbitraptor eightbitraptor Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@eregon eregon Feb 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


rss = get_rss
ruby_bench_results["rss"] = rss
if maxrss = get_maxrss
Expand Down
Loading