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

Stefan Johansson stefan.johansson at oracle.com
Thu Mar 29 09:53:25 UTC 2018


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 maybe_clear_bitmap_if_set(G1CMBitMap* bitmap, HeapWord* addr)

It will always clear if set, so I think we should rename it. What do you think about clear_bitmap_if_marked() or clear_mark_in_bitmap()?

 544   G1CollectorState* collector_state = _g1h->collector_state();  545   if (collector_state->mark_or_rebuild_in_progress() ||  546       collector_state->clearing_next_bitmap()) {  547     maybe_clear_bitmap_if_set(_next_mark_bitmap, 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?

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

Thanks, Stefan

Testing: local testing with existing tests (gc/g1/TestEagerReclaimHumongousRegionsClearMarkBits.java)

Thanks, Thomas



More information about the hotspot-gc-dev mailing list