RFR: 8155638: Resource allocated BitMaps are often cleared twice

[Stefan Karlsson]

Hi Dean,

On 2016-05-07 01:37, Dean Long wrote:
> Do we every call clear(), is_empty(), or is_full() twice in a row?  We 
> could add new "full" and "empty" flags so that we can quickly 
> determine if an operation is redundant.  If the code is complicated 
> enough, we might not find all cases from static analysis.

Some of my changes to BitMap has been done to minimize the size of the 
BitMap class. If we add such a flag I'd prefer if it was done in a 
subclass, or a wrapper class, if possible.

Thanks,
StefanK

>
> dl
>
>
> On 5/6/2016 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
>>
>