RFR: Add generations to freeset [v5]
Y. Srinivas Ramakrishna
ysr at openjdk.org
Thu Apr 13 20:39:00 UTC 2023
On Thu, 13 Apr 2023 20:21:03 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> Kelvin Nilsen has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Remove debug instrumentation
>
> src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp line 924:
>
>> 922: // We move all young available regions into mutator_free set and then we take back the regions we need for our
>> 923: // reserve. This allows us to "compact" the collector_free (survivor) regions at the high end of the heap.
>> 924: clear_mutator_free(idx);
>
> The `clear_mutator_free()` isn't consistent with the comment `move all young available regions into mutator_free set`.
>
> May be you can change the documentation comment to state:
>
> We examine each region in turn and assign it to the appropriate unique subset based on the following criteria:
> ... list of criteria used to assign to one or other of the subsets.
>
> Such a comment might help clarify the intent of the code below. Also, if the set abstraction were used with assertions moving into the set modification methods, then this method's logic and flow would be much easier to see. There is a leakage of abstraction happening here for the assertions because of the naked use of low-level constructs into the higher level logic of the algorithm for manipulating the set membership of the regions.
Actually, I see now that the `clear()` call further above have at line 916 already emptied all of the subsets (or at least that was the intent); thus, the individual removal from each of the subsets here isn't even necessary.
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1165991079
More information about the shenandoah-dev
mailing list