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