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

Jon Masamitsu jon.masamitsu at
Mon Nov 30 17:20:53 UTC 2015


On 11/26/2015 02:34 PM, Kim Barrett wrote:
> On Nov 25, 2015, at 5:10 PM, Jon Masamitsu <jon.masamitsu at> wrote:
>> I have a new fix for this bug.  My previous fix broke solaris-x86 (I
>> had not defined an early_initialize() for x86).  This fix is slightly
>> smaller and has the virtue of moving the required initialization
>> closer to where it is used.
>> Testing: JPRT build on all platforms, checked by hand that the correct number
>> of GC worker threads are created on later Niagara platforms.
>> Thanks.
> ------------------------------------------------------------------------------
> The new location for this setup (from os::init_before_ergo) is better
> than the earlier proposed change, which had it in
> VM_version::early_initialize.  The latter is incorrect, because
> determine_features (sparc) examines some command-line arguments, and
> those aren't parsed until a later stage of Threads::create_vm.
> Specifically, determine_features (sparc) is presently looking at two
> CLAs: UseV8InstrsOnly (develop) and UseNiagaraInstrs (product).  I
> suspect UseV8InstrsOnly has served its purpose and could be purged.
> But moving the call to determine_features before argument parsing
> would have unintentionally ignored UseNiagaraInstrs.
> The new location doesn't have this problem.  Sorry I missed this in my
> earlier review.
> ------------------------------------------------------------------------------
> src/cpu/x86/vm/vm_version_x86.hpp
>   595   static void vm_init_before_ergo() {};
> Trailing semi-colon is not needed.


> ------------------------------------------------------------------------------
> src/cpu/sparc/vm/vm_version_sparc.cpp
>    39   guarantee(VM_Version::has_v9(), "only SPARC v9 is supported");
>    40   assert(_features != VM_Version::unknown_m, "System pre-initialization is not complete.");
> I think the assert should be first.  The test performed by the
> guarantee is really only valid if the pre-initialization has been
> performed.

Agreed.  Fixed.
> ------------------------------------------------------------------------------
> 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.


> ------------------------------------------------------------------------------

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the hotspot-gc-dev mailing list