RFR: 8267834: Refactor G1CardSetAllocator and BufferNode::Allocator to use a common base class [v2]

Kim Barrett kbarrett at openjdk.java.net
Thu Feb 24 17:30:06 UTC 2022


On Thu, 24 Feb 2022 16:51:56 GMT, Ivan Walulya <iwalulya at openjdk.org> wrote:

>> Hi all,
>> 
>> Please review this change to combine free list allocation used in G1CardSetAllocator and BufferNode::Allocator into a single class. 
>> 
>> Testing: Tier 1-5
>> 
>> Thanks
>> Ivan
>
> Ivan Walulya has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleanup includes

Changes requested by kbarrett (Reviewer).

src/hotspot/share/gc/shared/freeListAllocator.cpp line 31:

> 29: 
> 30: FreeListAllocator::NodeList::NodeList() :
> 31:   _head(NULL), _tail(NULL), _entry_count(0) {}

NULL -> nullptr, here and elsewhere.  Some code here is already adopting nullptr, and since this is "new" it should be consistent.

src/hotspot/share/gc/shared/freeListAllocator.cpp line 82:

> 80:   while (list != NULL) {
> 81:     FreeNode* next = list->next();
> 82:     DEBUG_ONLY(list->set_next(NULL);)

Setting next to null isn't needed unless there's a null check in the destructor.  The current destructor is defaulted.  I think some predecessors to this code might have had a debug-only assert that next was null.

src/hotspot/share/gc/shared/freeListAllocator.cpp line 102:

> 100: // method on nodes not managed by an arena will leak the memory by just dropping
> 101: // the nodes to the floor.
> 102: void FreeListAllocator::reset() {

I prefer this kind of API documentation in the header.

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

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



More information about the hotspot-gc-dev mailing list