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

Hamlin Li mli at openjdk.java.net
Thu Nov 11 06:24:36 UTC 2021


On Wed, 10 Nov 2021 12:15:59 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix typo
>
> 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.

Thanks Thomas.

> I would prefer `G1SegmentedArrayFreePool` here, seeing the segmented array as the important thing (I understand that `G1SegmentedArrayBufferListFreePool` is too long). 

Sure.

> I think there is already a CR to reconsider naming of all these classes, 

Do you mean https://bugs.openjdk.java.net/browse/JDK-8276299 or some other issue?

> maybe add `G1SegmentedArrayBufferList` to the ones that need renaming; 

If you mean 8276299 above, sure I will add this to the renaming list of 8276299.

> in particular I think right now that the "BufferList" of `G1SegmentedArrayBufferList` is kind of redundant. 

Not quite get your point, would you mind to clarify further?

> Sorry for not speaking up earlier about this, but it only came to my mind right now.

It's OK, it's never late to make necessary changes. :)

> It would be great for that renaming CR to start a thread in hotspot-gc-dev before actually doing the renames. 

Do you mean not send a pr but just send email to hotspot-gc-dev to discuss this idea of renaming?

> 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.

If you mean 8276299 above, then I think these 2 (8276299 and this one) are not quite related to each other, 8276299 is to unify the naming of "buffer", "node" and "element" (especially in card set area currently), but this one is to factor out Factor out G1CardSetFreePool and enable evac failure to reuse the freeing memory logic in current G1CardSetFreePool.
Sure, I will start a discussion for 8276299 before actually send the pr.

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

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


More information about the hotspot-gc-dev mailing list