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

Kim Barrett kim.barrett at
Thu Nov 26 22:34:18 UTC 2015

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.

 595   static void vm_init_before_ergo() {};

Trailing semi-colon is not needed.

  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

 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.


More information about the hotspot-gc-dev mailing list