RFR: 6942632: Hotspot should be able to use more than 64 logical processors on Windows [v9]

[David Holmes]

On Wed, 13 Mar 2024 22:59:54 GMT, Saint Wesonga <duke at openjdk.org> wrote:

>> Windows processor groups support a maximum of 64 logical processors per processor group. OpenJDK currently uses the GetSystemInfo API to get the number of processors on the system. GetSystemInfo only returns the number of processors in the current processor group, which is at most 64. Therefore, OpenJDK needs to be updated to support the use of all available processors across all Windows processor groups. However, this should only be done when running on Windows 11 or Windows Server 2022 or later Windows operating systems.
>> 
>> Starting with Windows 11 and Windows Server 2022, applications are no longer constrained by default to a single processor group. Instead, processes and their threads have processor affinities that by default span all processors in the system, across multiple groups on machines with more than 64 processors. This is in contrast to previous operating system versions where the processor group affinity APIs needed to be used to assign each thread a specific processor group. See  https://learn.microsoft.com/en-us/windows/win32/procthread/processor-groups#behavior-starting-with-windows-11-and-windows-server-2022
>> 
>> The key changes required are:
>> 
>> 1. Detecting Windows 11 or Windows Server 2022 or later.
>> 1. Using the The GetLogicalProcessorInformationEx function to correctly retrieve the total number of processors available on the system. This function was introduced in Windows 7 so its availability needs to be explicitly determined.
>> 1. Updating the calculation of the number of active processors.
>>     1. Processor affinities can be set in a Windows job object and if the JVM is launched in a job object, these affinities restrict the processors that the JVM can use (regardless of how many processors the system has).
>>     1. The JVM can also be launched from the command prompt  by using the start command to set processor affinities instead of using a job object. This command only sets the processor affinity of the primary processor group because it is a 64-bit affinity mask. On systems with over 64 processors, passing an all 1s 64-bit affinity mask to the start command is treated as equivalent to no processor affinity restrictions for the JVM. Any other affinity mask passed to the start command explicitly sets the number of available processors for the JVM and is necessarily at most 63, even if the machine has more than 64 processors.
>>     1. If no affinity restrictions are present, then all cores on the system will be available t...
>
> Saint Wesonga has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Respect processor count from the job object

Sorry for the delay in getting back to this. There is a lot of API surface that I am not familiar with that I am trying to get my head around. The overall code structure and approach is looking very good - though I have one issue with the initialization process.

One thing that is missing though is a way to effectively test this given all the possible conditions that might be present:
- a job object exists, or not
- processor groups exist, or not
- affinity has been set, or not

I recognise it may be very difficult to create either a regression test or a gtest for this, but I would like to see some instructions on how to set up different scenarios and then check that `Runtime::availableProcessors()` reports what would be expected. Otherwise we have a ton of code here that is effectively untested.

I have made some suggestions for uniform logging statements so that we could use UL together with the test scenarios to check whether things are working as expected.

Thanks.

src/hotspot/os/windows/os_windows.cpp line 4150:

> 4148:   }
> 4149: 
> 4150:   return processors;

Before returning please add a `log_debug(os)` statement indicating that there is an active job object and the number of processors it defines.

src/hotspot/os/windows/os_windows.cpp line 4193:

> 4191:   }
> 4192: 
> 4193:   return logical_processors;

Before returning please add a `log_debug(os)` statement indicating the number of logical processors discovered.

src/hotspot/os/windows/os_windows.cpp line 4208:

> 4206:   DWORD processors = 0;
> 4207:   bool schedules_all_processor_groups = win32::is_windows_11_or_greater() || win32::is_windows_server_2022_or_greater();
> 4208:   if (schedules_all_processor_groups) {

Please add a `log_debug(os)` statement indicating that this version does schedule all processor groups.

src/hotspot/os/windows/os_windows.hpp line 84:

> 82:     initialize_windows_version();
> 83:     return _build_minor;
> 84:   }

There should only be one path that initializes the version info and we should be very clear what that one path is. Can these methods be called before the call to `os::init` that calls `initialize_system_info` that calls `initialize_windows_version`? If so we should fix that. Otherwise these methods should assert the version is initialized.

Then `initialize_windows_version` should assert that it is not already initialized.

-------------

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17576#pullrequestreview-1944929493
PR Review Comment: https://git.openjdk.org/jdk/pull/17576#discussion_r1529601420
PR Review Comment: https://git.openjdk.org/jdk/pull/17576#discussion_r1529601945
PR Review Comment: https://git.openjdk.org/jdk/pull/17576#discussion_r1529602591
PR Review Comment: https://git.openjdk.org/jdk/pull/17576#discussion_r1529586108