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

[Stefan Johansson]

Hi Thomas and Sangheon,

I have one comment on Thomas comments =)

On 2019-10-04 14:11, Thomas Schatzl wrote:
> Hi Sangheon,
> 
>    thanks for your  hard work on this!
> 
> On 01.10.19 18:43, sangheon.kim at oracle.com wrote:
>> Hi Kim and others,
>>
>> This webrev.2 simplified a bit more after changing 'heap expansion' 
>> approach.
> [...]
>> webrev:
>> http://cr.openjdk.java.net/~sangheki/8220310/webrev.2/
>> http://cr.openjdk.java.net/~sangheki/8220310/webrev.2.inc
>> Testing: hs-tier1 ~ 5 +-UseNUMA
>>
> 
> Comments:
> 
> ...
> 
> - in some discussion we talked about the "node_index" lifecycle, and 
> what I remember is the following:
> 
>    - initially, when we commit/make the region available, we set that 
> HeapRegion's node_index to "Unknown" (with AlwaysPretouch on we can of 
> course immediately set the correct one).
>    - in HeapRegion::node_index() we do something like the following 
> pseudo-code:
> 
>    {
>      if (_node_index == Unknown) {
>        // try to get actual node index from OS, and update _node_index 
> if we could get the information
>      }
> 
>      if (_node_index == Unknown) { // Still unknown
>        // return _preferred_ node index *without* updating _node_index
>      }
>      return _node_index;
>    }
> 
>    - now, during the "verification" pass, we use whether 
> HeapRegion::node_index() == preferred_node_index to determine if the 
> region is on the correct node.
> 
> The change only sets the node index during making the region available, 
> and immediately to the preferred node index.
> 
> 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 
update during a safepoint. The second thing is that if pages get 
migrated by the OS we would not see this if we only request the actual 
node index one time.

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

Thanks,
Stefan