RFR: 8325808: GenShen: Move generational mode code out of shFullGC.cpp

William Kemper wkemper at openjdk.org
Mon Feb 26 21:53:57 UTC 2024


On Mon, 19 Feb 2024 18:21:36 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> This change reduces the differences from the upstream branch by moving large chunks of generational mode code into separate files.
>
> src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 460:
> 
>> 458: 
>> 459: template<typename ClosureType>
>> 460: void ShenandoahPrepareForCompactionTask::prepare_for_compaction(ClosureType& cl,
> 
> Looking at this method, it looks like it actually belongs as a public method in the closure, with that closure's methods invoked here becoming private to the closure. Makes for a narrower public API all around and keeps the loop in what appears to me in its more natural place.

Hmm, I agree from a design perspective. However, we have two versions of the closure. For them to share the implementation of `prepare_for_compaction` as you suggest, I believe we'd need to introduce a common base class (or parent one closure to another) and make some of these methods virtual. Or were you thinking of making `prepare_for_compaction` a static member of a common base class and keeping the same signature. Or, just duplicating the implementation of `prepare_for_compaction` in each closure? Not sure if any of those are simpler than what we have here.

> src/hotspot/share/gc/shenandoah/shenandoahGeneration.hpp line 126:
> 
>> 124:   size_t available() const override;
>> 125:   size_t available_with_reserve() const;
>> 126:   size_t used_and_wasted() const {
> 
> Nit: technically `used_or_wasted()`, since space that is used is not wasted.

How about `used_including_waste`?

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

PR Review Comment: https://git.openjdk.org/shenandoah/pull/398#discussion_r1503346149
PR Review Comment: https://git.openjdk.org/shenandoah/pull/398#discussion_r1503347497


More information about the shenandoah-dev mailing list