RFR: 8224816: Provide os::processor_id() implementation for Mac OS

[Erik Österlund]

Hi Per,

Thanks for the review.

/Erik

On 2019-05-29 00:53, Per Liden wrote:
> Looks good.
>
> /Per
>
> On 05/28/2019 10:34 AM, Erik Österlund wrote:
>> Hi Per,
>>
>> Thanks for reviewing.
>>
>> On 2019-05-28 09:46, Per Liden wrote:
>>> Hi Erik,
>>>
>>> On 5/27/19 12:05 PM, Erik Österlund wrote:
>>>> Hi,
>>>>
>>>> In order to port ZGC to Mac OS, we need to implement 
>>>> os::processor_id(), due to the use of CPU locals in ZGC.
>>>> I built an implementation that uses CPUID to get the APIC id, and 
>>>> from that determine a unique CPU id.
>>>> Note though that the APIC ids are split over a potentially larger 
>>>> number space than the number of processors.
>>>> In order to reliably translate APIC ids to CPU ids, I first use 
>>>> CPUID to query how many bits are being used
>>>> for APIC ids, and then create an array of integers covering that 
>>>> space. The array maps possible APIC id positions
>>>> to a unique processor id. The array is lazily populated with APIC 
>>>> id -> processor id mappings.
>>>> The specific CPUID leaf used to query this topology information is 
>>>> intel-specific, but that should be okay as all macs are Intel based.
>>>> I have tried this out on a number of macs with a mac port of ZGC 
>>>> (to be upstreamed soon), and it worked well.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8224816
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~eosterlund/8224816/webrev.00/
>>>
>>> Just a few comments:
>>>
>>> * Maybe replace "cpu" with "processor" in variable names, since the 
>>> function is named processor_id().
>>
>> Fixed.
>>
>>> * Could we split out the setup of the array into a separate function 
>>> helper, like:
>>>
>>>   volatile int* mapping = get_apic_to_processor_mapping();
>>
>> Fixed.
>>
>>> * Move the declaration of "apic_to_cpu_mapping" and "next_cpu_id" 
>>> out of the function, to avoid the pthread_once initialization checks 
>>> every call?
>>
>> Fixed.
>>
>> New webrev:
>> http://cr.openjdk.java.net/~eosterlund/8224816/webrev.01/
>>
>> Incremental:
>> http://cr.openjdk.java.net/~eosterlund/8224816/webrev.00_01/
>>
>> Thanks,
>> /Erik
>>
>>> cheers,
>>> /Per
>>>
>>>>
>>>> Thanks,
>>>> /Erik
>>