RFR: 8276670: G1: Move and rename G1CardSetFreePool and related classes [v3]

Thomas Schatzl tschatzl at openjdk.java.net
Wed Nov 10 12:20:38 UTC 2021


On Mon, 8 Nov 2021 03:12:12 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Currently, only the card sets (remembered sets) use G1CardSetFreePool to give back memory to OS.
>> 
>> After JDK-8254739, this memory reclaiming mechanism could be reused by evacuation failure too. This is a preparation change to allow reuse of this code.
>> 
>> I plan to do this in about 3 steps to smooth the review process:
>>   1. move G1CardSetFreePool and related classes to new file, rename these classes
>>   2. refactor these classes to support freeing other freelist
>>   3. some necessary cleanup
>> 
>> This is to simply move and rename G1CardSetFreePool and related classes, as G1CardSetFreePool and related classes are going to be reused outside of the remembered set, they should be factored out and renamed.
>> Rename from G1CardSetXxx -> G1BufferListXxx.
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix typo

Changes requested by tschatzl (Reviewer).

src/hotspot/share/gc/g1/g1BufferListFreeMemoryTask.hpp line 36:

> 34: 
> 35: // Task handling deallocation of free card set memory.
> 36: class G1BufferListFreeMemoryTask : public G1ServiceTask {

Comment needs to be adapted.

src/hotspot/share/gc/g1/g1BufferListFreeMemoryTask.hpp line 57:

> 55: 
> 56:   // Current total card set memory usage.
> 57:   G1BufferListMemoryStats _total_used;

Comment mentions "card set"

src/hotspot/share/gc/g1/g1BufferListFreePool.cpp line 114:

> 112:   _first = cur;
> 113: 
> 114:   log_trace(gc, task)("Card Set Free Memory: Returned to VM %zu buffers size %zu", keep_num, keep_size);

"Card Set"

src/hotspot/share/gc/g1/g1BufferListFreePool.hpp line 58:

> 56: 
> 57: // A set of free lists holding memory buffers for use by G1CardSetAllocators.
> 58: template<MEMFLAGS flag>

G1CardSetAllocators

src/hotspot/share/gc/g1/g1BufferListFreePool.hpp line 59:

> 57: // A set of free lists holding memory buffers for use by G1CardSetAllocators.
> 58: template<MEMFLAGS flag>
> 59: class G1BufferListFreePool {

I would prefer `G1SegmentedArrayFreePool` here, seeing the segmented array as the important thing (I understand that `G1SegmentedArrayBufferListFreePool` is too long).
I think there is already a CR to reconsider naming of all these classes, maybe add `G1SegmentedArrayBufferList` to the ones that need renaming; in particular I think right now that the "BufferList" of `G1SegmentedArrayBufferList` is kind of redundant.
Sorry for not speaking up earlier about this, but it only came to my mind right now.

It would be great for that renaming CR to start a thread in hotspot-gc-dev before actually doing the renames. Assuming that this renaming will be a close follow-up I do not have a particular opinion about the current name chosen, but maybe it is useful to actually start this discussion now to avoid renaming this class in particular again.

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

PR: https://git.openjdk.java.net/jdk/pull/6289


More information about the hotspot-gc-dev mailing list