RFR: 8155638: Resource allocated BitMaps are often cleared twice

[Stefan Karlsson]

Thanks, Vladimir!

StefanK

On 2016-06-03 02:08, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 5/6/16 3:46 AM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to remove some redundant clearing of bitmaps.
>>
>> http://cr.openjdk.java.net/~stefank/8155638/webrev.01
>> https://bugs.openjdk.java.net/browse/JDK-8155638
>>
>>
>> It used to be the case that Resource allocated bitmaps were _not_ 
>> cleared in the constructor, and that explicit calls to clear() were 
>> needed. However, when G1 was open sourced all the Resource
>> allocated bitmaps were always cleared in the BitMap constructor:
>>
>> http://hg.openjdk.java.net/jdk9/hs/hotspot/diff/37f87013dfd8/src/share/vm/utilities/bitMap.cpp 
>>
>>
>>
>> It's apparent that the lazy clearing of bitmaps was used to skip 
>> clearing when it wasn't necessary.
>>
>> See, for example:
>> http://cr.openjdk.java.net/~stefank/8155638/webrev.01/src/share/vm/c1/c1_IR.cpp.udiff.html 
>>
>>
>> where _requires_phi_function is cleared even when all bits are going 
>> to be set to 1.
>>
>> and:
>> http://cr.openjdk.java.net/~stefank/8155638/webrev.01/src/share/vm/compiler/methodLiveness.cpp.frames.html 
>>
>>
>> in MethodLiveness::BasicBlock::get_liveness_at
>>
>> 1010   ResourceBitMap g(_gen.size()); g.set_from(_gen);
>> 1011   ResourceBitMap k(_kill.size()); k.set_from(_kill);
>>
>> where we first clear the bitmap, then copy all bits with set_from. 
>> Before G1 was introduced, the unnecessary clearing was not done.
>>
>> I've created a new CR for these kind of issues:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8156207 - Resource allocated 
>> BitMaps are often cleared unnecessarily
>>
>>
>> During the review of "8141501: Problems with BitMap buffer 
>> management", I got the request do the following change:
>>
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-May/022969.html
>>
>> I tried it, but felt that the change didn't fit the surrounding code, 
>> where shallow-copying was used for other bitmaps. It would be great 
>> if reviewers of this patch could decide if I should change the
>> code as suggested.
>>
>>
>> Thanks,
>> StefanK
>>