RFR: 8325673: GenShen: Share Reserves between Old and Young Collector [v6]

[Kelvin Nilsen]

On Wed, 24 Jul 2024 15:07:18 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Improve comment
>
> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 106:
> 
>> 104:   size_t _fragmentation_last_old_region;
>> 105: 
>> 106:   // State variables involved in construction of a mixed-evacuation collection set
> 
> Can you add a sentence describing when the state is updated, and when it is used, as well as which are inputs to the construction process and which the results of (or updated upon completion of) the construction process and used as inputs to downstream operations?

Thanks.  Done.

> src/hotspot/share/gc/shenandoah/heuristics/shenandoahOldHeuristics.hpp line 141:
> 
>> 139:   void choose_collection_set_from_regiondata(ShenandoahCollectionSet* set, RegionData* data, size_t data_size, size_t free) override;
>> 140: 
>> 141:   // Return true iff we need to finalize mixed evacs
> 
> In addition to the return value, please add a sentence stating something like "Adds old regions to the collection set." What does "need to finalize mixed evacs" mean? Did you mean: "Returns true if and only if this step should be followed by ... (see method finalize_mixed_evacs)." In general, it's better to write comments such that they do not require chains of lookup (although in some cases it may be unavoidable).

Yes.  Very good suggestions.  Done.

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/395#discussion_r1693495597
PR Review Comment: https://git.openjdk.org/shenandoah/pull/395#discussion_r1693495840