[ping] Re: [11] RFR(M): 8189922: UseNUMA memory interleaving vs membind

[Thomas Schatzl]

Hi,

On Wed, 2018-07-18 at 19:46 -0300, Gustavo Romero wrote:
> Hi Community,
> 
> In the light of the additional information brought by Derek and Swati
> on this issue (which I summarized below) would it be possible to get
> a second assessment for this issue regarding its priority? Would it
> be sound to qualify it as P3 instead of P4? I don't have experience
> on doing such an assessment so I ask.
> 
> - If the JVM isn't allowed to use memory on all of the numa nodes,
> for instance, by numactl, cgroups, and docker container, then a
> significant fraction of the JVM heap will be unusable, causing early
> GC;
> 
> - The waste can be up to (N-1)/N of the young generation in some
> cases, where N is the total number of nodes available on the system
> (unpinned); So on an EPYC machine with 8 numa nodes, for instance,
> waste can be up to 7/8 of total memory available on a given node;
> 
> - With the patch for this issue applied SPECjbb2015 on a EPYC machine
> (8 NUMA nodes) shows a significant performance improvement:
> 
>    Case 1: Performance for 1 NUMA node bound, MultiJVM 1 Group Run
> (numactl --cpunodebind=0 --membind=0)
>      Max-jOPS      : +27.59%
>      Critical-jOPS : +260% (As base memory without patch is 1/8 of
> total available memory, heap size impacts Critical-jOPS)
> 
>    Case 2: Performance for 2 NUMA nodes bound, Composite Run (numactl
> --cpunodebind=0,7 --membind=0,7)
>      Max-jOPS      : +10.35%
>      Critical-jOPS : +9.89%
> 
> - It affects AARCH64, PPC64, and Intel/AMD.
> 

  I believe the priority could be increased given these numbers. I will
ask other triaging people what they think too.

Not expecting any issues, I will run it through our test infra (that
does not use non-default node binding) too.

I looked at the webrev, some comments:

- the v2 (http://cr.openjdk.java.net/~gromero/8189922/v2/) webrev does
not apply cleanly any more.

- but it seems good except for some odd naming (imo): the getters
returning a bool do not have the "is" prefix followed by an underscore.
Is that somehow intentional or related to the libnuma naming? (the
"isnode_..." / "isbound_..." prefixes which I would call "is_node_..."
and "is_bound_..") I saw that the numa methods introduced in (I think)
your other patch do the same, so I am not sure. In any case it is no
real issue, and could be cleaned up later too.

I can sponsor to push the change into whatever JDK it ends up.

Thanks,
  Thomas