RFR: 8276670: G1: Rename G1CardSetFreePool and related classes
Albert Mingkun Yang
ayang at openjdk.java.net
Wed Nov 24 15:19:08 UTC 2021
On Tue, 23 Nov 2021 06:26:19 GMT, Hamlin Li <mli at openjdk.org> wrote:
> As G1CardSetFreePool and related classes are going to be reused outside of the remembered set, they should be renamed.
Some minor comments/suggestions.
src/hotspot/share/gc/g1/g1CardSetMemory.cpp line 208:
> 206: for (uint i = 0; i < num_mem_object_types(); i++) {
> 207: result._num_mem_sizes[i] += _allocators[i].mem_size();
> 208: result._num_segments[i] += _allocators[i].num_buffers();
I see a few other places where `buffer` is *not* renamed to `segment`. Having a dedicated PR for `buffer` -> `segment` renaming can increase the cohesion of each PR, IMO.
src/hotspot/share/gc/g1/g1CardSetMemory.hpp line 147:
> 145: public:
> 146: G1CardSetMemoryManager(G1CardSetConfiguration* config,
> 147: G1SegmentedArrayFreePool<mtGCCardSet>* free_list_pool);
Instead of using `G1SegmentedArrayFreePool<mtGCCardSet>` explicitly, I think the current name, `G1CardSetFreePool`, is more meaningful in this context (user site).
Therefore, I wonder if it's possible to place `using G1CardSetFreePool = G1SegmentedArrayFreePool<mtGCCardSet>` somewhere and keep using the current name.
src/hotspot/share/gc/g1/g1SegmentedArrayFreeMemoryTask.hpp line 86:
> 84:
> 85: public:
> 86: explicit G1SegmentedArrayFreeMemoryTask(const char* name);
Looking at the call-site and implementation of `execute()` method, I am not sure this class is generic enough to be called `G1SegmentedArrayFreeMemoryTask`. (Maybe this class will be used for sth other than Card Set in the near future?)
src/hotspot/share/gc/g1/g1SegmentedArrayFreePool.hpp line 39:
> 37:
> 38: size_t _num_mem_sizes[G1CardSetConfiguration::num_mem_object_types()];
> 39: size_t _num_segments[G1CardSetConfiguration::num_mem_object_types()];
It's a bit odd to have `G1CardSetXXX` embedded inside this class. Can be fixed in a followup PR.
src/hotspot/share/gc/g1/g1SegmentedArrayFreePool.hpp line 62:
> 60: class G1SegmentedArrayFreePool {
> 61: // The global free pool.
> 62: static G1SegmentedArrayFreePool<flag> _freelist_pool;
I think one can drop `<flag>` here and in other similar places
-------------
PR: https://git.openjdk.java.net/jdk/pull/6514
More information about the hotspot-gc-dev
mailing list