Request for Review (s) - 8133023: ParallelGCThreads is not calculated correctly

[Jon Masamitsu]

David,


On 12/04/2015 08:57 PM, David Holmes wrote:
> Hi Jon,
>
> On 5/12/2015 2:39 AM, Jon Masamitsu wrote:
>> Here's a new version of the fix (simpler thanks to a suggestion
>> from Kim).
>>
>> http://cr.openjdk.java.net/~jmasa/8133023/webrev.05/
>
> Yes this seems quite neat. It is a pity about 
> yet-another-VM_Version-init-function but ...
>
> Only minor comment is that in VM_Version::vm_init_before_ergo the "vm" 
> part is somewhat unnecessary and doesn't fit with the existing: 
> initialize() and early_initialize(). How about just 
> initialize_before_ergo()?

I dropped the "vm_" from the name.

>
> Also we now really need a comment explaining when the plain 
> initialize() function is used.

Is this along the lines of what you had in mind?

   // Called as part of the runtime services initialization
   // called from the management module initialization.  Examines
   // a variety of the hardware capabilities of the platform
   // to determine which features can be used to execute the
   // program.
   static void initialize();

Jon

>
> Thanks,
> David
> -----
>
>> In the above webrev vm_version_x86.hpp appears but has no changes.
>> There were changes to it in the previous version (04) but were undone in
>> this latest version and may be why it appears in the webrev.
>>
>> The delta webrev is
>>
>> http://cr.openjdk.java.net/~jmasa/8133023/delta.05/
>>
>> It has a white-space change in vm_version_sparc.hpp  which
>> does not appear in the diffs.
>>
>> Thanks.
>>
>> Jon
>>
>>
>> On 12/2/2015 9:46 AM, Jon Masamitsu wrote:
>>>
>>>
>>> On 11/30/2015 11:47 AM, Kim Barrett wrote:
>>>> On Nov 30, 2015, at 12:20 PM, Jon Masamitsu
>>>> <jon.masamitsu at oracle.com> wrote:
>>>>>> src/share/vm/runtime/os.cpp
>>>>>>   319   VM_Version::vm_init_before_ergo();
>>>>>>
>>>>>> This call is in generic code, but only two definitions have been
>>>>>> provided, for sparc and x86.  Missing are aarch64, ppc, and zero.
>>>>>>
>>>>> Add vm_init_before_ergo() for those even though JPRT does not
>>>>> build them? I don't need to be convinced to do it.  Just
>>>>> encouraged.  Say "do it" and I'll do it.
>>>> I think you should do it, rather than knowingly breaking the build
>>>> for those targets when the needed code is so trivial.
>>>>
>>>
>>> Here's the webrev for the delta for review comments to this point.
>>>
>>> http://cr.openjdk.java.net/~jmasa/8133023/delta.04/
>>>
>>> Webrev for complete patch is here.
>>>
>>> http://cr.openjdk.java.net/~jmasa/8133023/webrev.04/
>>>
>>> Jon
>>