RFR: Add generations to freeset [v21]

Y. Srinivas Ramakrishna ysr at openjdk.org
Mon May 1 17:00:24 UTC 2023


On Sat, 29 Apr 2023 14:22:53 GMT, Kelvin Nilsen <kdnilsen at openjdk.org> wrote:

>> ShenandoahFreeSet has not yet been modified to deal efficiently with the combination of old-gen and young-gen collection set reserves.  This PR makes changes so that we can distinguish between collector_is_free, old_collector_is_free, and mutator_is_free.  Further, it endeavors to keep each set of free regions tightly packed, so the range of regions representing each set is small.
>> 
>> In its current form, this no longer fails existing regression tests (except for known problems that are being addressed independently)
>
> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Respond to reviewer feedback

I didn't go through all of the details of the allocation strategies to reduce entropy of memory types across the range of regions etc., but this looks good otherwise.

Would you be able to share some statistics or performance numbers that indicate the improvement here, either using range statistics or just raw allocation performance (say using a hyperalloc/extremem workload to compare)?

Thanks for patiently responding to my nitpickings during the review process!

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 272:

> 270:   // request.  Regions become sparsely distributed following a Full GC, which tends to slide all regions to the front of the
> 271:   // heap rather than allowing survivor regions to remain at the high end of the heap where we intend for them to congregate.
> 272:   // In the future, we may modify Full GC so that it slides old objects to the end of the heap and young objects to the start

Mark with a `TODO:` ?

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 92:

> 90:   // In other words:
> 91:   //   if the requested which_set is empty:
> 92:   //     left_most() and left_most_empty() return _max, right_most() and right_most_empty() return 0

I think you an elide the "_" underscore in left_most (leftmost is an English word). It also makes `leftmost_empty` easier to read.

Would be great if you can make that global substitution in your comments etc. as well before checking in. (Or make it all have the underscore consistently in comments too if you prefer the variant with the underscore.)

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 124:

> 122: 
> 123:   // Assure leftmost, rightmost, leftmost_empty, and rightmost_empty bounds are valid for all free sets.
> 124:   // valid bounds honor all of the following (where max is the number of heap regions):

Valid (upper-case V).

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

Marked as reviewed by ysr (Author).

PR Review: https://git.openjdk.org/shenandoah/pull/250#pullrequestreview-1407804254
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1181709565
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1181705647
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1181700452


More information about the shenandoah-dev mailing list