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

David Holmes david.holmes at oracle.com
Sat Dec 5 04:57:43 UTC 2015


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()?

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

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
>


More information about the hotspot-dev mailing list