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

[Jon Masamitsu]

On 11/17/2015 02:31 PM, Kim Barrett wrote:
> 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:

Yes, that would be good.  What about the guarantee().  Some of the other 
variables depend on
has_v9().

Jon


>
>