Request for review (S): 8006242: G1: WorkerDataArray<T>::verify() too strict for double calculations
john.cuthbertson at oracle.com
Wed Jan 16 18:10:30 UTC 2013
Excellent. Ship it.
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.
>> Check the indentation of the for loop in G1GCPhaseTimes::note_gc_end().
>> On 1/14/2013 1:06 PM, Bengt Rutisson wrote:
>>> Hi all,
>>> Could I have a couple of reviews for this small change?
>>> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev