RFR: 8258431: Provide a JFR event with live set size estimate [v9]

Jaroslav Bachorik jbachorik at openjdk.java.net
Mon Mar 8 17:29:30 UTC 2021


On Wed, 3 Mar 2021 12:03:01 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Jaroslav Bachorik has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add tests for the heap usage summary event
>
> src/hotspot/share/gc/g1/g1CollectedHeap.hpp line 182:
> 
>> 180:   G1BlockOffsetTable* _bot;
>> 181: 
>> 182:   volatile size_t _live;
> 
> I'm not happy with naming this `_live`, better use `_live_estimate`. The contents are not continuously updated and basically out of date after the first following allocation.
> This includes the naming in all other instances too.

I see your point - but that would probably lead to renaming `live()` method to `live_estimate()` (to keep the variable and the accessor method in sync) and that would break the nice symmetry we have now with `free()`, `used()` and `live()`. 
I have no strong feelings about this and if we can get quorum on this change I will do the renaming pass.

> src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp line 87:
> 
>> 85: 
>> 86:   // in order to provide accurate estimate this method must be called only when the heap has just been collected and compacted
>> 87:   inline void capture_live();
> 
> Sentences should start with upper case in the comment. Also I'd prefer to name the method `update_live_estimate()` instead.

Done

> src/hotspot/share/gc/parallel/psAdaptiveSizePolicy.hpp line 60:
> 
>> 58: class PSAdaptiveSizePolicy : public AdaptiveSizePolicy {
>> 59:  friend class PSGCAdaptivePolicyCounters;
>> 60:  friend class ParallelScavengeHeap;
> 
> Delete this apparently unneeded friend declaration (compiled successfully without here)

Done

> src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 1070:
> 
>> 1068: 
>> 1069:     uint num_selected_for_rebuild() const { return _num_regions_selected_for_rebuild; }
>> 1070:     size_t live_estimate() const { return _live; }
> 
> Please sync the member name with the getter name. I.e. `_live` -> `_live_estimate`

Done

> src/hotspot/share/gc/serial/serialHeap.hpp line 44:
> 
>> 42:   MemoryPool* _old_pool;
>> 43: 
>> 44:   size_t _live_size;
> 
> Please rename to `_live_estimate` like the others. Avoid having different names in different collectors for the same thing.

This field is unused. Removed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2579



More information about the hotspot-gc-dev mailing list