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

[Kim Barrett]

> On Oct 23, 2019, at 12:20 PM, sangheon.kim at oracle.com wrote:
> 
> Hi Per,
> 
> Thanks for taking a look at this.
> 
> I agree all your comments and here's the webrev.
> - All comments from Per.
> - Move G1PageBasedVirtualSpace::page_size() near to page_start() from Kim.
> 
> Webrev:
> http://cr.openjdk.java.net/~sangheki/8220310/webrev.6
> http://cr.openjdk.java.net/~sangheki/8220310/webrev.6.inc
> Testing: build test for linux, solaris, windows and mac.
> 
> FYI, as I think existing numa related API names and -1 stuff seem not good, I planned to refine those later after pushing. But as you said following existing rule and then refine all together later seems better.

The type of the argument for numa_get_group_id(void* address) should
be "const void*".  Sorry I didn't notice that earlier.  Of course,
this will require a const_cast to remove the const qualifier when
calling get_mempolicy, but it is better to isolate the workaround for
that missing qualifier to that one place.

I'm not sure I like the overload for os::numa_get_group_id.  While
both are getting the numa id associated with something, the associations
involved seem pretty different to me.

Spelling them out, they could be

numa_get_group_id_for_current_thread()
numa_get_group_id_for_address(const void* address)

Those seem semantically unrelated to me, so violate the usual guidance
of only overloading operations that are roughly equivalent (*).  Or put
another way, one should not need to determine which overload is selected
to understand a call site.

Of course, "roughly equivalent" is in the eye of the beholder.

(*) Operator overloading sometimes violates this on the basis that the
syntactic concision of using operators is more important, and there
are a limited set of operators.  Such violations are often used as an
argument against using operator overloading at all.