RFR(XL): 8220310: Implementation: NUMA-Aware Memory Allocation for G1, Mutator (1/3)

[Kim Barrett]

> On Oct 11, 2019, at 1:34 PM, sangheon.kim at oracle.com wrote:
> On 10/10/19 4:34 PM, Kim Barrett wrote:
>> src/hotspot/share/gc/g1/g1Allocator.cpp
>>  186   assert(Heap_lock->owner() != NULL, "Should be owned on this thread's behalf.");
>> 
>> Use assert_lock_strong(Heap_lock).
> It didn't work.
> assert_lock_string() checks "lock->owned_by_self()" which is not equivalent to "lock::owner() != NULL". Am I missing something?
> 
> Since this is pre-existing code, I would like to leave as is.

Oh, bleh, you are right.  I didn’t read the existing code carefully enough.

>> src/hotspot/share/gc/g1/heapRegion.cpp
>>  464     st->print("|Node ID %02d", node_ids[this->node_index()]);
>> 
>> The unchecked use of node_index() here can run afoul of an unset (so
>> UnknownNodeIndex) index.
> Added such checking.
>> 
>> Also, no need for `this->` in `this->node_index()`.
> Removed.
> I'm aware but tried to follow local style which uses 'this->' in that code.

There is one other use of this-> in that function (and one additional one in the whole file).
The *vast* majority of accesses use the implicit this.  So I wouldn’t describe that as the
local style, rather a couple of weirdnesses that probably should be cleaned up.

> webrev:
> http://cr.openjdk.java.net/~sangheki/8220310/webrev.4
> http://cr.openjdk.java.net/~sangheki/8220310/webrev.4.inc
> 
> Testing: hs-tier 1 ~ 5 with/without UseNUMA

I’ve started looking at the new webrev.  Looking good, and no comments yet, but not done yet either.