RFR (S): 8200385: Eagerly reclaimed humongous objects leave mark in prev bitmap (original) (raw)

Thomas Schatzl thomas.schatzl at oracle.com
Thu Mar 29 10:55:22 UTC 2018

On Thu, 2018-03-29 at 11:53 +0200, Stefan Johansson wrote:

On 2018-03-29 11:12, Thomas Schatzl wrote: > Hi all, > > can I have reviews for this small fix to a benign bug, that is I > haven't seen any actual product failure from it but some very rare > test failures, where when we eagerly reclaim humongous objects we > leave a mark on the prev bitmap in some cases? > > The suggested fix is to always look at the prev bitmap and clear > it, and if needed also clear potential marks in the next bitmap. > > To make the failure appear basically 100% in that test, I added a > simple assert after reclaiming the humongous object. > > With the fix, this failure goes away completely. > > Note that this is more a "data structure hygiene" fix - the stray > mark on the prev bitmap will be automatically cleared after > switching the bitmaps at cleanup and preparing that bitmap for the > next mark. > > CR: > https://bugs.openjdk.java.net/browse/JDK-8200385 > Webrev: > http://cr.openjdk.java.net/~tschatzl/8200385/webrev/ Good fix, even though the problem is benign it is always nice to have a consistent state. A few comments: src/hotspot/share/gc/g1/g1ConcurrentMark.cpp 532 static void maybeclearbitmapifset(G1CMBitMap* bitmap, HeapWord* addr) It will always clear if set, so I think we should rename it. What do you think about clearbitmapifmarked() or clearmarkinbitmap()?

I changed it to clear_mark_if_set.

544 G1CollectorState* collectorstate = g1h->collectorstate(); 545 if (collectorstate->markorrebuildinprogress() || 546 collectorstate->clearingnextbitmap()) { 547 maybeclearbitmapifset(nextmarkbitmap, r->bottom()); 548 } I get that this might be a bit more efficient, but I would prefer always clearing both bitmaps, to not have to depend on the state. Or would there be any problem with that?

I do not think so. I will change that as suggested.

There is also a difference here that will now clearstatistics even if markorrebuildinprogress() returns false. Is this on purpose? I don't see any problem with this just wanted to check.

Not on purpose, it does not matter. The statistics are not used beyond the mark_or_rebuild_in_progress() part. I will revert to the old code.

http://cr.openjdk.java.net/~tschatzl/8200385/webrev.1 (full only; sorry I messed up the incremental diff)

Thanks, Thomas

More information about the hotspot-gc-dev mailing list