Request for review (s) - 8145031: Add regression test for 8133023

[Jon Masamitsu]

Tom,

The other reason I wanted to add the test was that I was surprised
by the failure.  The order of initialization was moved around at
some point in jdk8 and created this failure.   I could have
done some assertion checking when the GC threads were
created but that would have been during JVM initialization
and I wanted to get completely past initialization and then
do the checking.  I don't expect initialization  to be moved
around so much that GC thread initialization would be done
before cpu identification but I didn't expect the first bug
could happen either.

Jon


On 12/16/2015 02:45 PM, Jon Masamitsu wrote:
> Tom,
>
> Thanks for looking at this.
>
> On 12/16/2015 07:45 AM, Tom Benson wrote:
>> Hi Jon,
>> This looks OK to me (modulo Thomas' comments), but it's not clear to 
>> me why a unit test is needed for this case, rather than a simple 
>> assert somewhere, such as when the threads are created. Thanks,
>
> Do you mean add an assertion when each GC thread is created?
>
> I think that would add checks to 2 places (one for ParallelGC and
> one for the other GC's - ignoring the concurrent threads).  Adding
> the check at one place seemed better.
>
> Or maybe you meant someplace else?
>
> Jon
>
>> Tom
>>
>> On 12/15/2015 2:01 PM, Jon Masamitsu wrote:
>>>
>>>
>>> On 12/15/2015 2:24 AM, Thomas Schatzl wrote:
>>>> Hi Jon,
>>>>
>>>> On Thu, 2015-12-10 at 11:50 -0800, Jon Masamitsu wrote:
>>>>> The cause of the failure in 8133023 was the calculation of the
>>>>> ParallelGCThreads using
>>>>> information that had not yet been initialized. Add a test that checks
>>>>> that, after full
>>>>> initialization, the fresh calculation of ParallelGCThreads is
>>>>> consistent with the value
>>>>> calculated earlier.
>>>>>
>>>>> http://cr.openjdk.java.net/~jmasa/8145031/webrev.00/
>>>>>
>>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8145031
>>>>>
>>>>> Tested with and without the fix for 8133023.
>>>>>
>>>>    seems okay, with one nit: the indendation of the parameters for the
>>>> assert in Abstract_VM_Version::FinalParallelGCThreads_test() seems 
>>>> off.
>>>>
>>>> Also there is an extra space between the "void" and the method name.
>>>
>>> Both fixed.  Thanks.
>>>
>>> Jon
>>>
>>>>
>>>> I do not need to re-review.
>>>>
>>>> Thanks,
>>>>    Thomas
>>>>
>>>>
>>>
>>
>