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

[Bengt Rutisson]

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/eb5fce76/attachment.htm>