RFR: 8243961: ForceNUMA and only one available NUMA node fails assertion on Windows

[Thomas Schatzl]

Hi,

On 05.05.20 08:24, Kim Barrett wrote:
>> On May 4, 2020, at 5:43 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>> On 02.05.20 11:59, Kim Barrett wrote:
>>> [Added ppc-aix-port-dev, as there is a small AIX change here.]
>> [...]
>>> What I've ended up doing is moving the conditional enabling of UseNUMAInterleaving into the platform-specific code, where there's enough information to consistently get it right. I've also made all the os variants be explicit about UseNUMA and UseNUMAInterleaving; those that have no support for either now unconditionally set them false.  (This happens to keep TestUseNUMAInterleaving working on
>>> those platforms.)
>>> New webrev: https://cr.openjdk.java.net/~kbarrett/8243961/open.01/
>>> (No incremental; the code change to os_windows.cpp is the same, but the rationale and commentary are entirely different.)
>>
>> os_linux.cpp: s/explicilty/explictly (pre-existing, may ignore)
>>
>> There is an inconsistency in the use of FLAG_SET_ERGO vs. directly setting flag values when the implementation does not support NUMA. E.g. on bsd FLAG_SET_ERGO is used to disable UseNUMA*, on Windows it is directly set to false when not setting ForceNUMA.
>>
>> Since ForceNUMA is "soon" going away it's not a big issue for me though.
> 
> I think the places that are setting UseNUMA based on ForceNUMA aren't
> "ergonimic"; they are doing exactly what the user directly requested.
> If ForceNUMA were removed I'd make those places use FLAG_SET_ERGO.
> 
>>> Testing: mach5 tier1-3, normally, with -XX:+UseNUMA -XX:+ForceNUMA
>>> added.
>>> TestUseNUMAInterleaving still fails on Windows with +UseNUMA +ForceNUMA on a single-node machine, because +UseNUMAInterleaving
>>> gets turned off, which is not what the test expects.
>>
>> One option could be to add a @requires vm.opt.ForceNUMA != true to avoid unnecessary failures.
> 
> I thought about that but prefer to leave the test alone, rather than
> (perhaps temporarily) conditionalize for an option that is being
> deprecated.
> 

Okay, all good.

Thomas