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

Bengt Rutisson bengt.rutisson at oracle.com
Wed Jan 16 12:18:05 UTC 2013


Hi Vitaly,

Thanks for looking at this!

On 1/15/13 2:03 PM, Vitaly Davidovich wrote:
>
> Hi Bengt,
>
> Looks good.  Do you need the constants for int/double/size_t? Would it 
> be easier to have a static getter that returns "(T)-1" and then use 
> that instead?
>

It would work, but I'm not sure what is best. I prefer the constants, 
but I would be fine with a getter too. If you don't have strong 
objections I'll leave it as it is.

Thanks,
Bengt

> Thanks
>
> Sent from my phone
>
> On Jan 14, 2013 4:08 PM, "Bengt Rutisson" <bengt.rutisson at oracle.com 
> <mailto:bengt.rutisson at oracle.com>> wrote:
>
>
>     Hi all,
>
>     Could I have a couple of reviews for this small change?
>     http://cr.openjdk.java.net/~brutisso/8006242/webrev.00/
>     <http://cr.openjdk.java.net/%7Ebrutisso/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.java.net/pipermail/hotspot-gc-dev/attachments/20130116/dab8ba73/attachment.htm>


More information about the hotspot-gc-dev mailing list