Request for review (S): 8006242: G1: WorkerDataArray<T>::verify() too strict for double calculations

[John Cuthbertson]

Hi Bengt,

Excellent. Ship it.

JohnC

On 1/16/2013 4:01 AM, Bengt Rutisson wrote:
>
> Hi John,
>
> Thanks for the review!
>
> On 1/15/13 6:59 PM, John Cuthbertson wrote:
>> Hi Bengt,
>>
>> Changes look good to me. Minor nits:
>>
>> Copyrights need updating.
>
> I'll leave the copyright year as is for now. There is an ongoing 
> discussion about whether or not we need to do this. I'd prefer to wait 
> and see what the decision is.
>
>> Use UINT32_FORMAT instead of %d in the error message.
>
> Done.
>
>> Check the indentation of the for loop in G1GCPhaseTimes::note_gc_end().
>
> Done.
>
> Bengt
>
>>
>> JohnC
>>
>> On 1/14/2013 1:06 PM, Bengt Rutisson wrote:
>>>
>>> Hi all,
>>>
>>> Could I have a couple of reviews for this small change?
>>> http://cr.openjdk.java.net/~brutisso/8006242/webrev.00/
>>>
>>> Thanks to John Cuthbertson for finding this bug and providing 
>>> excellent data to track down the issue.
>>>
>>> From the bug report:
>>>
>>> In non-product builds the WorkerDataArrays in G1 are initialized to 
>>> -1 in WorkerDataArray<T>::reset() when a GC starts. At the end of a 
>>> GC WorkerDataArray<T>::verify() verifies that all entries in a 
>>> WorkerDataArray has been set. Currently it does this by asserting 
>>> that the entries are >= 0. This is fine in theory since the entries 
>>> should contain counts or times that are all positive.
>>>
>>> The problem is that some WorkerDataArrays are of type double. And 
>>> some of those are set up through calculations using doubles. If 
>>> those calculations result in a value close to 0 we could end up with 
>>> a value slightly less than 0 since double calculations don't have 
>>> full precision.
>>>
>>> All we really want to verify is that all the entries were set. So, 
>>> it should be enough to verify that entries do not contain the value 
>>> set by the reset() method.
>>>
>>> Bengt
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130116/47a010f2/attachment.htm>