RFR: 8232651: Add implementation of os::processor_id() for Windows

[Stefan Karlsson]

Hi David,

On 2019-10-22 01:17, David Holmes wrote:
> Hi Stefan,
> 
> This fix looks good.
> 
> One comment below ...
> 
> On 22/10/2019 6:24 am, Stefan Karlsson wrote:
>> Hi Thomas,
>>
>> On 2019-10-21 16:34, Thomas Stüfe wrote:
>>> Hi Stefan,
>>>
>>> looks good to me.
>>
>> Thanks.
>>
>>>
>>> Note that according to the documentation we should have to include 
>>> processthreadsapi.h. I wonder why this works without it.
>>
>> Good catch. My guess is that it gets included by one of the other 
>> included header files. I'll add processthreadsapi.h and make sure that 
>> it compiles as expected.
> 
> It isn't needed as you just include Windows.h to get it:
> 
> "Header     processthreadsapi.h (include Windows Vista, Windows 7, 
> Windows Server 2008 Windows Server 2008 R2, Windows.h)"

OK. I thought that meant that you should include Windows.h on Vista, 7, 
Server 2008, Server 2008 R2, and include processthreadsapi.h on newer 
releases.

I tried to compile the following in a separate cpp file:

#include <Windows.h>

DWORD test_GetCurrentProcessorNumber() {
   return GetCurrentProcessorNumber();
}

and it succeeded. The include patch to processthreadsapi.hpp were:

  Windows.h
   winbase.h
    processthreadsapi.h

So, I'll proceed with David's suggestion to not add processthreadsapi.h.

Thanks for the review,
StefanK

> 
> Thanks,
> David
> 
>> Thanks for reviewing,
>> StefanK
>>
>>>
>>> Cheers, Thomas
>>>
>>>
>>> On Mon, Oct 21, 2019 at 4:23 PM Stefan Karlsson 
>>> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>> wrote:
>>>
>>>     Hi all,
>>>
>>>     Please review this patch to add an implementation of
>>>     os::processor_id()
>>>     for Windows.
>>>
>>>     https://cr.openjdk.java.net/~stefank/8232651/webrev.01/
>>>     https://bugs.openjdk.java.net/browse/JDK-8232651
>>>
>>>     We need an implementation of this function on Windows, to be able
>>>     to run
>>>     ZGC there.
>>>
>>>     Note that GetCurrentProcessorNumber only "returns the processor
>>>     number
>>>     within the processor group to which the logical processor is
>>>     assigned".
>>>     AFAICT, there's no support for multi-groups in HotSpot, and 
>>> therefore
>>>     this seems like an adequate function to use.
>>>
>>>     See this page for more information about processor groups:
>>> https://docs.microsoft.com/en-us/windows/win32/procthread/processor-groups 
>>>
>>>
>>>     Thanks,
>>>     StefanK
>>>
>>