RFR (S): 8147910: Cache initial active_processor_count

[David Holmes]

Updates look good Thomas!

Thanks,
David

On 25/07/2016 6:13 PM, Thomas Schatzl wrote:
> Hi David,
>
> On Mon, 2016-07-25 at 14:55 +1000, David Holmes wrote:
>> Hi Thomas,
>>
>> Thanks again for taking this on.
>
>   thank you for the quick reviews.
>
>>
>> On 22/07/2016 9:36 PM, Thomas Schatzl wrote:
>>>
>>> Hi all,
>>>
>>>   can I have reviews for this tiny change that adds an API to set
>>> the
>>> initial active processor count at startup? It also adds that count
>>> to
>>> the hs_err log file.
>>>
>>> It is required to fix JDK-8161993.
>>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8147910
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~tschatzl/8147910/webrev/
>> src/share/vm/runtime/os.hpp
>>
>> Nit: can you change update_initial_active_processor_count() to
>> initialize_initial_active_processor_count() please - otherwise it
>> gives
>> the impression that it can be called many times to update the value.
>>
>> +     guarantee(_initial_active_processor_count > 0, "Initial active
>> processor count not set yet.")
>>
>> An assert will suffice here.
>>
>
> All fixed.
>
>> ---
>>
>> src/share/vm/runtime/os.cpp:
>>
>> !   // We access the raw value here because using the accessor will
>> let
>> its guarantee
>> !   // fail if the crash occurs before initialization of this value.
>> !   st->print(" (initial active %d)",
>> _initial_active_processor_count);
>>
>> Good point! Did you find that out the hard way? :)
>>
>> I wonder if we should clarify the message in that case:
>>
>> (initial active %d - zero if not yet set)
>
>   I think it would be just confusing in the most common case. There is
> always the comment in the code that already indicates that a zero value
> means not initilialized yet.
>
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8147910/webrev.0_to_1 (diff)
> http://cr.openjdk.java.net/~tschatzl/8147910/webrev.1 (full)
>
>
> Thanks,
>   Thomas
>