Feature #18364: Add GC.stat_pool for Variable Width Allocation

• Status: Closed
• Assignee: matz (Yukihiro Matsumoto)
• Target version: -

[peterzhu2118 (Peter Zhu)]

GitHub PR: https://github.com/ruby/ruby/pull/5177

We're proposing an API to get statistics for size pools for Variable Width Allocation similar to GC.stat. This will make it easier for us (and other developers) to tune VWA.

Before 3.1 release, we plan to keep this method hidden from the documentation using :nodoc: since it is not useful when not using VWA.

For example:

# Get stats for size pool 2
puts GC.stat_pool(2)
#=> {:slot_size=>160, :heap_allocatable_pages=>80, :heap_eden_pages=>14, :heap_eden_slots=>1424, :heap_tomb_pages=>0, :heap_tomb_slots=>0}
puts GC.stat_pool(2, :heap_eden_pages)
#=> 14

We aim to keep the keys in the outputted hash the same as the keys used in GC.stat.

We chose to implement a new method instead of re-using an existing API (GC.stat) because the keys returned by GC.stat_pool will not be the same as GC.stat. We believe that having GC.stat return different shapes of hashes based on its arguments is confusing.

[peterzhu2118 (Peter Zhu)]

• Description updated (diff)

[peterzhu2118 (Peter Zhu)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

There is something maybe related in TruffleRuby and JRuby, where we have per-heap information.
Maybe it would make sense to have a unified interface for per-heap information?

TruffleRuby:

$ ruby -e 'pp GC.heap_stats'
{:time=>29,
 :count=>4,
 :minor_gc_count=>4,
 :major_gc_count=>0,
 :unknown_count=>0,
 :used=>26931712,
 :heap_live_slots=>26931712,
 :committed=>530579456,
 :heap_available_slots=>530579456,
 :heap_free_slots=>503647744,
 :init=>526385152,
 :max=>8420065280,
 "G1 Eden Space"=>
  {:used=>0,
   :committed=>75497472,
   :init=>27262976,
   :max=>-1,
   :peak_used=>46137344,
   :peak_committed=>327155712,
   :peak_init=>27262976,
   :peak_max=>-1,
   :last_used=>0,
   :last_committed=>75497472,
   :last_init=>27262976,
   :last_max=>-1},
 :peak_used=>73069056,
 :peak_committed=>834666496,
 :peak_init=>526385152,
 :peak_max=>8420065278,
 :last_used=>8388608,
 :last_committed=>83886080,
 :last_init=>526385152,
 :last_max=>8420065278,
 "G1 Survivor Space"=>
  {:used=>8388608,
   :committed=>8388608,
   :init=>0,
   :max=>-1,
   :peak_used=>8388608,
   :peak_committed=>8388608,
   :peak_init=>0,
   :peak_max=>-1,
   :last_used=>8388608,
   :last_committed=>8388608,
   :last_init=>0,
   :last_max=>-1},
 "G1 Old Gen"=>
  {:used=>18543104,
   :committed=>446693376,
   :init=>499122176,
   :max=>8420065280,
   :peak_used=>18543104,
   :peak_committed=>499122176,
   :peak_init=>499122176,
   :peak_max=>8420065280,
   :last_used=>0,
   :last_committed=>0,
   :last_init=>499122176,
   :last_max=>8420065280}}

JRuby:

ruby -e 'pp GC.stat'                   
{:count=>2,
 :time=>17,
 :committed=>1061158912.0,
 :init=>1052770304.0,
 :max=>16840130556.0,
 :used=>44554240.0,
 :peak_committed=>1660944384.0,
 :peak_init=>1052770304.0,
 :peak_max=>16840130556.0,
 :peak_used=>65525760.0,
 :last_committed=>79691776.0,
 :last_init=>1052770304.0,
 :last_max=>16840130556.0,
 :last_used=>8388608.0,
 "G1 Young Generation"=>
  {:count=>2,
   :time=>17,
   :pools=>
    {"G1 Old Gen"=>
      {:committed=>490733568,
       :init=>499122176,
       :max=>8420065280,
       :used=>7597056,
       :peak_committed=>499122176,
       :peak_init=>499122176,
       :peak_max=>8420065280,
       :peak_used=>7597056,
       :last_committed=>0,
       :last_init=>499122176,
       :last_max=>8420065280,
       :last_used=>0}}},
 "G1 Old Generation"=>
  {:count=>0,
   :time=>0,
   :pools=>
    {"G1 Old Gen"=>
      {:committed=>490733568,
       :init=>499122176,
       :max=>8420065280,
       :used=>7597056,
       :peak_committed=>499122176,
       :peak_init=>499122176,
       :peak_max=>8420065280,
       :peak_used=>7597056,
       :last_committed=>0,
       :last_init=>499122176,
       :last_max=>8420065280,
       :last_used=>0}}}}

[peterzhu2118 (Peter Zhu)]

Thanks for the TruffleRuby and JRuby info @Eregon (Benoit Daloze)! Maybe we could extend GC.stat to include size pool info in an array. For example:

pp GC.stat
{:count=>21,
 :heap_allocated_pages=>99,
...
 :size_pools=>
  [
   {:slot_size=>40,
    :heap_allocatable_pages=>4,
    :heap_eden_pages=>40,
    :heap_eden_slots=>1234,
    :heap_tomb_pages=>0,
    :heap_tomb_slots=>0},
   {:slot_size=>80,
    :heap_allocatable_pages=>80,
    :heap_eden_pages=>14,
    :heap_eden_slots=>123,
    :heap_tomb_pages=>0,
    :heap_tomb_slots=>0}
   ...
  ],
...
}

However, I think I think that this would be a breaking change for those that assume all the values in the hash are of type Numeric.

[Eregon (Benoit Daloze)]

I would avoid extending GC.stat, I think it is good it only contains Symbols to Integer, and there is probably code relying on that.
Also GC.stat(key) seems relatively performance-sensitive as it's used in Rails for every request.

I think a new method is good.
Maybe GC.heap_stats/pool_stats/stat_pool` or so?

I think it would be useful if this new method when given no arguments return a Hash of all heaps/memory pools, so it also serves as an easy way to list heaps/memory pools.
And with an argument, only the stats for that specific heap/memory pool, and with 2 arguments the given stat for that memory pool.

That way, whether a heap/memory pool is identified by a size or a name it would work in both cases.

[peterzhu2118 (Peter Zhu)]

I like the idea of having a no argument case that returns stats for all pools.

In the ticket description, I named the method GC.stat_size_pool. I now feel that the name is too MRI implementation specific and wouldn't work well for other implementations. I like your suggestion of GC.stat_pool.

[byroot (Jean Boussier)]

I think it is good it only contains Symbols to Integer, and there is probably code relying on that.

Yes, size_t rb_gc_stat(VALUE) is public for C exts. So you can't even return a Float, or you'd have to refactor the code significantly to make these keys different or invisible from the C side.

[peterzhu2118 (Peter Zhu)]

• Subject changed from Add GC.stat_size_pool for Variable Width Allocation to Add GC.stat_pool for Variable Width Allocation
• Description updated (diff)

[mame (Yusuke Endoh)]

This ticket was discussed at today's dev-meeting. Matz had directly asked @tenderlovemaking (Aaron Patterson) for a few things. I record them as far as I remember:

• Instead of adding a new method, it looks good for GC.stat to return a nested data structure format like #4.
• The terminology "pool" looks not very good. How about "size_heap"?

[peterzhu2118 (Peter Zhu)]

Thank you for the summary @mame (Yusuke Endoh)! We'll extend GC.stat to return a nested data structure.

[Eregon (Benoit Daloze)]

peterzhu2118 (Peter Zhu) wrote in #note-10:

Thank you for the summary @mame (Yusuke Endoh)! We'll extend GC.stat to return a nested data structure.

This seems incompatible and might break existing code which expects GC.stat to return a Hash[Symbol,Integer].
Also what should size_t rb_gc_stat(VALUE) return/raise for such a case?
Also it's likely to make GC.stat()/GC.stat(hash) slower and use more allocations, which seems suboptimal.
Notably, GC.stat(hash) no longer avoids allocations and becomes useless.

I don't understand @matz's reasoning here, the OP and myself agreed GC.stat_pool is best and it covers both the VWA use-case and JRuby/TruffleRuby.
The nested approach seems to have multiple issues, and a key like size_heap seems too specific (those details might change over time) and makes no sense for other Ruby implementations.
Maybe it was missed in #4 that it's the result of GC.heap_stats on TruffleRuby, not of GC.stat?

"memory pool" is a well established term in this area.
Another method name is stat_heap if that sounds clearer.

[peterzhu2118 (Peter Zhu)]

This seems incompatible and might break existing code which expects GC.stat to return a Hash[Symbol,Integer].

Quoting the dev meeting log:

ko1: Also jruby already extends this method so that it returns more than just a number.
matz: If so I guess we can follow jruby?

Personally, I don't have a strong opinion on which route we should take (whether we extend GC.stat or add a new method).

Also what should size_t rb_gc_stat(VALUE) return/raise for such a case?

I think we should change API for rb_gc_stat to return VALUE instead. There probably isn't many users of this API so it shouldn't be a breaking change for many.

Also it's likely to make GC.stat()/GC.stat(hash) slower and use more allocations, which seems suboptimal.
Notably, GC.stat(hash) no longer avoids allocations and becomes useless.

We can still achieve this by checking that the non-numeric values of the key are of the correct type and re-using it.
Although, this would significantly increase the complexity of the code for GC.stat.

[Eregon (Benoit Daloze)]

meeting log is https://github.com/ruby/dev-meeting-log/blob/master/DevelopersMeeting20211209Japan.md

@ko1 (Koichi Sasada) said:

GC.stat returns a flat hash (Symbol to Numeric map) now. This proposal changes it so that the method returns a nested hash.

But that is not this proposal or the description of this ticket.
@ko1 (Koichi Sasada) were you confused by this ticket description maybe?
I think this is why the resolution is so far from what was discussed here: it seems the description of this issue wasn't read properly and understood.

To make it clear, breaking existing usages of GC.stat which assume it's Integer/Numeric values will bring compatibility issues, and they will only appear late when VWA is made default.
I.e., that would be a significant breaking change, and so it makes no sense to me.

peterzhu2118 (Peter Zhu) wrote in #note-12:

I think we should change API for rb_gc_stat to return VALUE instead. There probably isn't many users of this API so it shouldn't be a breaking change for many.

That seems very risky, because size_t and VALUE are both integer types, so code using it wouldn't notice the change but would likely just the print the nested Hash address as an integer which is unwanted.
I think the only possibility here is to raise for that C API if the value is not an Integer.

We can still achieve this by checking that the non-numeric values of the key are of the correct type and re-using it.
Although, this would significantly increase the complexity of the code for GC.stat.

How so? By having the caller do something like h = { size_heap: {} }; GC.stat?
But the caller code shouldn't need to know about this key.
Or maybe you mean h = GC.stat; GC.stat(h)? I'm not sure it's a common pattern.

[peterzhu2118 (Peter Zhu)]

If callers want to prevent the probe effect from calls to GC.stat, they have to not only create a hash beforehand, but also do a call to GC.stat.
Since the first call can trigger GC, I don't think it's unreasonable to also allocate objects.

my_hash = {}
# This call is needed to initialize the hash with keys
# Adding keys to the hash can cause the hash to resize which can trigger GC
GC.stat(my_hash)
# This call to GC.stat now guarantees that the probe effect won't happen
GC.stat(my_hash)

[Eregon (Benoit Daloze)]

I forgot to mention, I think GC.stat is expected to be reasonably fast as e.g. it might be done on every request for monitoring.

OTOH capturing per-heap information is typically significantly slower (at least it is on JVM), and so providing that information in a separate method avoids making GC.stat slower.

peterzhu2118 (Peter Zhu) wrote in #note-14:

If callers want to prevent the probe effect from calls to GC.stat, they have to not only create a hash beforehand, but also do a call to GC.stat.
Since the first call can trigger GC, I don't think it's unreasonable to also allocate objects.

That's a fair point, and probably somewhat already needed as otherwise the Hash will typically need some extra (internal) allocations to fit enough entries.
I think we should document this better in the GC.stat docs.

[peterzhu2118 (Peter Zhu)]

Thank you for all the comments @Eregon (Benoit Daloze). I think no one else has strong opinions on extending GC.stat,
and you've presented some very good reasons to not extend GC.stat, so I think we'll add an additional
API for accessing statistics for memory pools. Since @matz (Yukihiro Matsumoto) is not a fan of the word "pool", we plan on
renaming the API GC.stat_heap.

[peterzhu2118 (Peter Zhu)]

• Status changed from Open to Closed

Applied in changeset git|615e9b28658c5b44a4474e04a53b84ae83b8e3fd.

---

[Feature #18364] Add GC.stat_heap to get stats for memory heaps

GC.stat_heap will return stats for memory heaps. This is
used for the Variable Width Allocation feature.