RFR: 8155638: Resource allocated BitMaps are often cleared twice

[Dean Long]

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.

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
>