Request for review (XS): 7197906: BlockOffsetArray::power_to_cards_back() needs to handle > 32 bit shifts

[Srinivas Ramakrishna]

Great, thanks John, all!

-- ramki

On Fri, Sep 14, 2012 at 12:19 AM, John Cuthbertson <
john.cuthbertson at oracle.com> wrote:

> **
> Hi Ramki,
>
> I don't think there will be an issue with back porting this to hs24 (which
> I think is going into 7u12). I just have to check the dates. There's a
> couple of other CRs that we're probably going to want to back port as well.
>
> I searched the sources for the offending pattern (with and without
> whitespace) and only found the instances fixed by Hal. I think we're OK in
> that respect. But thanks for making sure. :)
>
> JohnC
>
>
> On 09/13/12 15:21, Srinivas Ramakrishna wrote:
>
>
> Thanks Bengt; changes look good to me too, Hal, although i just looked at
> the changes, and did not
> check the code to see if there might be more instances of the fingered
> anti-pattern. (I am pretty sure
> you and John and other reviewers have covered that front well :-)
>
> Bengt, any chance that the fix will get bkported also to 7uXX soon, for
> some suitable value of XX?
>
> thanks!
> -- ramki
>
> On Thu, Sep 13, 2012 at 8:27 PM, Bengt Rutisson <bengt.rutisson at oracle.com
> > wrote:
>
>>
>> Hi Ramki,
>>
>> Thanks for looking at this!
>>
>> Here's a webrev based on the second patch that Hal sent out:
>> http://cr.openjdk.java.net/~brutisso/7197906/webrev.02/
>>
>> Bengt
>>
>>
>> On 2012-09-13 18:12, Srinivas Ramakrishna wrote:
>>
>> Hal, thanks for the catch and the fix!
>>
>> Bengt, might it be possible to host this as a traditional webrev in the
>> usual place, to simplify review logistics?
>>
>> thanks!
>> -- ramki
>>
>> On Thu, Sep 13, 2012 at 4:55 PM, Jianhao Mo <mojianhao at gmail.com> wrote:
>>
>>> Hi all,
>>>
>>>  Attached please find the new patch, modified according to John's
>>> advice.
>>>
>>>  Thanks Bengt again for the help :)
>>>
>>>  Regards,
>>>
>>>  Hal
>>>
>>> 2012/9/12 Jianhao Mo <mojianhao at gmail.com>
>>>
>>>> Hi all,
>>>>
>>>>  Could I get a couple of reviews for this small patch, please?
>>>>
>>>>  7197906: BlockOffsetArray::power_to_cards_back() needs to handle > 32
>>>> bit shifts
>>>>
>>>>  CR link:
>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7197906
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~brutisso/7197906/webrev.01/
>>>> Contributed-by:
>>>>  Hal Mo <kungu.mjh at taobao.com> from Alibaba Group(with OCA)
>>>>
>>>>  It may take a while before the CR link is publicly accessible.
>>>>
>>>>  In blockOffsetTable.hpp, there is an unexpected data truncation in
>>>> power_to_cards_back(uint i), which may lead to crashing the VM on very
>>>> large GC heaps.
>>>> The problem could be reproduced easily on machines, that have enough
>>>> memory, with:
>>>>  $ java -Xmx135g -XX:+UseConcMarkSweepGC
>>>>
>>>>   The proposed patch fixes this problem, and builds on all OpenJDK
>>>> supported platforms.
>>>>
>>>>  I'd like to thank Bengt Rutisson for the initial review and help
>>>> preparing the CR and Webrev.
>>>>
>>>>  Regards,
>>>> Hal
>>>
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20120914/8365a37f/attachment.htm>