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