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

Hamlin Li mli at openjdk.java.net
Thu Nov 11 09:22:35 UTC 2021


On Thu, 11 Nov 2021 09:08:38 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> 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.
>
>> 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?
> 
> Yes.
> 
>> 
>> > 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?
> 
> A segmented array is an array consisting of segments (as opposed to a single huge array of something); a list of buffers, where a "buffer" is nothing but an array (of some element type) is almost the same. One is a set of chunks of elements (to explicitly use other words), the other too.
> 
>> > 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?
> 
> Not to discuss the idea of renaming, 8276299 is pretty clear about that, but discuss the actual mappings from old name to new name. Maybe this will wake up more people to the discussion than hiding it in a PR :) 
> 
>> 
>> > 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.
> 
> That's true, but we are renaming something that also contains a "Buffer" in it and seemingly related.
> 
> So instead of renaming this `G1BufferListFreePool` again to something else in 8276299, maybe we could save the time and immediately change it to the final name now, then in 8276299 do the same for the other classes.
> 
> Then 8276299 will hopefully go through more smoothly too if we already decided on the new names beforehand as well.

Thanks for clarification, I see your point, will initiate an discussion later. :)

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

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


More information about the hotspot-gc-dev mailing list