RFR: Add generations to freeset [v5]
Kelvin Nilsen
kdnilsen at openjdk.org
Wed Apr 19 14:20:18 UTC 2023
On Thu, 13 Apr 2023 20:24:16 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:
>> 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.
I've removed the redundant clearing and added comments to clarify this loop's behavior.
-------------
PR Review Comment: https://git.openjdk.org/shenandoah/pull/250#discussion_r1171414404
More information about the shenandoah-dev
mailing list