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

Jon Masamitsu jon.masamitsu at oracle.com
Wed Dec 16 22:45:38 UTC 2015


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




More information about the hotspot-gc-dev mailing list