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