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

[Thomas Schatzl]

Hi,

On 07.10.19 10:25, Stefan Johansson wrote:
> Hi Thomas and Sangheon,
> 
> I have one comment on Thomas comments =)
> 
> On 2019-10-04 14:11, Thomas Schatzl wrote:
>> Hi Sangheon,
[...]
>>
>> I.e. we eventually end up with the actual node index reported by the 
>> OS in HeapRegion::_node_index.
>>
> 
> I like the idea of being able to get the correct node index from 
> HeapRegion, but I have two concerns about the above idea. First, this 
> will cause us to do a syscall while holding the lock to get a new 
> region. This might not be a big deal, but I would prefer to do this
I am not completely into the actual code flow right now, but I do not 
think there is a need to get the node index in this code path from the 
HeapRegion. Maybe when allocating from the free list later?

> update during a safepoint. The second thing is that if pages get 

Fine with me too to piggyback it on some existing region iteration to be 
100% sure.

> migrated by the OS we would not see this if we only request the actual 
> node index one time.

This is what the logging/verification is for I guess at this time. If 
the migration is significant, we need to handle this and update the node 
index - but I think we can do this node index update as RFE.

Above update of the actual node index values during safepoint could also 
"always" do the summary logging then (with gc+numa=debug or something) 
if NUMA is enabled.

Overall I would agree with that too.

> It's possible that both those concerns can be ignored, but I wanted to 
> bring them up to hear others opinions.

Thanks,
   Thomas