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

[Stefan Johansson]

Hi Sangheon,

On 2019-10-15 16:33, sangheon.kim at oracle.com wrote:
> Hi all,
> 
> Here's revised webrev which addresses:
> 1) G1RegionToSpaceMapper checks mtJavaHeap and then conditionally calls 
> G1NUMA::request_memory_on_node() (Kim)
> 2) The signature of G1NUMA::request_memory_on_node(void* address, ,) is 
> changed to have actual address instead of page index. (Stefan)
> 3) Some local variable name changes at G1RegionToSpaceMapper. i -> 
> region_idx, idx -> page_idx (for local style, used idx instead of index)
> 
> webrev:
> http://cr.openjdk.java.net/~sangheki/8220310/webrev.5/
> http://cr.openjdk.java.net/~sangheki/8220310/webrev.5.inc/

This looks good!

Thanks for all your hard work,
Stefan

> Testing: hs-tier 1 ~ 5, with/without UseNUMA
> 
> Thanks,
> Sangheon
> 
> 
> On 10/14/19 3:20 PM, Kim Barrett wrote:
>>> On Oct 14, 2019, at 5:03 PM, Kim Barrett <kim.barrett at oracle.com> wrote:
>>>> 2. Add a state to the mappers to say if they are NUMA aware or not, 
>>>> and currently only the heap mapper should be NUMA aware. We could 
>>>> either set this state to true using the mtJavaHeap type as we have 
>>>> checked before or add an explicit setter that we only call for the 
>>>> heap mapper.
>>>>
>>>> I know that only doing 2) will fix the current problem, but I think 
>>>> it would be nice to avoid having the base address in G1NUMA, thoughts?
>>> I don't understand the point about mappers needing to know if they are
>>> NUMA or not. request_memory_on_node is only called by the two relevant
>>> region->space mappers, with the memory involved always in the Java
>>> heap (after fixing the units mismatch mentioned above). That is,
>>> G1NUMA::request_memory_on_node should only be called for Java heap
>>> memory. (It might be able to assert is_in_reserved or something like
>>> that, though initialization order might prevent that.)
>> I was confused here too.  Sangheon has repaired my confusion, and he’s
>> got another change in the works to tidy things up here in a way that I 
>> think
>> will make both me and Stefan happy.
>>
>