RFR: Add generations to freeset [v10]
Y. Srinivas Ramakrishna
ysr at openjdk.org
Fri Apr 21 22:18:53 UTC 2023
On Fri, 21 Apr 2023 18:41:36 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:
>
> Add TODO comment for work on recompute_bounds
src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp line 37:
> 35: OldCollector
> 36: };
> 37:
Did you consider avoiding templates and having a FreeSet class with the relevant methods `in_set(idx)`, `add_to_set(idx)`, etc. that you instantiate three of, a mutator, collector, and old_collector. Then, instead of `in_set<Mutator>(idx)`, etc. you would do `mutator.in_set(idx)`, etc. The downside is that we you can't then do assertion checks about membership of other sets (i.e. mutual exclusion checks) inside the methods that add to a specific set, so would need some wrapper machinery for that outside in ShenandoahFreeSet.
However, the advantage is that such a FreeSet can hide your interval bounds adjustment and checks nicely. Also, that kind of encapsulation allows your `assert_bounds()` to just do `mutator.assert_bounds(); collector.assert_bounds(); ...` etc.
One question was why the addition of a single element doesn't just do the interval adjustment.
add(x) {
if (x < leftmost) {
leftmost = x;
} else if (x > rightmost) {
rightmost = x;
} // else isn't extremal
set_bit(x);
}
Similarly, the removal of an element also doing the interval adjustment, although more expensive:
remove(x) {
if (x == leftmost) {
for (i = leftmost+1; i <= rightmost; i++) {
if (is_set_bit(i)) {
leftmost = i;
break;
}
}
assert(leftmost > x, "Error");
} else if (x == rightmost) {
for (i = rightmost - 1; i >= leftmost; i--) {
if (is_set_bit(i)) {
rightmost = i;
break;
}
}
assert(rightmost < x, "Error");
}
}
unset_bit(x);
}
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1174195086
More information about the shenandoah-dev
mailing list