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

[Kim Barrett]

On Nov 17, 2015, at 4:12 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
> 
> 
> 
> On 11/17/2015 12:00 PM, Kim Barrett wrote:
>> On Nov 12, 2015, at 4:31 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>>> GC calls VM_Version::calc_parallel_worker_threads() to determine
>>> the number of GC threads to create.  On Sparc it checks for newer
>>> Niagara hardware to decide the proper scaling of the GC threads with
>>> the hardware threads.  calc_parallel_worker_threads() was being called
>>> before enough information was gathered to determine the Sparc hardware.
>>> 
>>> Moved the gathering of information needed to earlier in the JVM
>>> initialization.
>>> 
>>> http://cr.openjdk.java.net/~jmasa/8133023/webrev.00/
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8133023
>> ------------------------------------------------------------------------------
>> src/cpu/sparc/vm/vm_version_sparc.hpp
>>  155   static bool is_niagara_plus()         { assert(_features != 0, "System initialization is not complete."); return is_T_family(_features) && !is_T1_model(_features); }
>> 
>> I agree with Thomas, _features should be compared with unknown_m
>> rather than 0 in the assert.
>> 
>> But that assert seems oddly placed and lonely.  If that _feature check
>> has an assert, why don't all the others?  To me that assert seems like
>> leftover development/debug code at this point.
> 
> It's actually an afterthought.
> 
> I've moved it here.
> 
> http://cr.openjdk.java.net/~jmasa/8133023/webrev.01/

Shouldn’t the assert be at the beginning of VM_Version::initialize, rather than some distance into it.
There are some _features-based predicates being called before that assert on line 63: