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

[Kim Barrett]

On Nov 25, 2015, at 5:10 PM, Jon Masamitsu <jon.masamitsu at oracle.com> 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.
> 
> http://cr.openjdk.java.net/~jmasa/8133023/webrev.03/index.html
> 
> 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.

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

------------------------------------------------------------------------------