RFR (M): 8151126: Clean up duplicate code for clearing the mark bitmaps (original) (raw)

Thomas Schatzl thomas.schatzl at oracle.com
Fri Mar 11 13:19:56 UTC 2016

Hi Jon,

On Thu, 2016-03-10 at 10:42 -0800, Jon Masamitsu wrote:

http://cr.openjdk.java.net/~tschatzl/8151126/webrev/src/share/vm/gc/g 1/g1ConcurrentMark.cpp.frames.html

728 void G1ConcurrentMark::clearprevbitmap(WorkGang* workers) { 729 assert(SafepointSynchronize::isatsafepoint(), "Should only clear the entire prev bitmap at a safepoint."); 730 clearbitmap((G1CMBitMap*)prevMarkBitMap, workers, false); 731 } Would it be appropriate to move the assertion at 729 to 698 void G1ConcurrentMark::clearbitmap(G1CMBitMap* bitmap, WorkGang* workers, bool mayyield) { assert(mayyield ||SafepointSynchronize::isatsafepoint(), "Should only clear the entire bitmap at a safepoint." 699 G1ClearBitMapTask task(bitmap, this, workers->activeworkers(), mayyield); 700 workers->runtask(&task); 701 guarantee(!mayyield || task.iscomplete(), "Must have completed iteration when not yielding."); 702 } Seems like you would get more coverage when needed for the assertion.

I think these are separate concerns: that clear_prev_bitmap is not called outside a safepoint, and that clear_bitmap() in general is not called without yielding outside of a safepoint. (Not sure if that mean with "additional coverage").

New webrevs: http://cr.openjdk.java.net/~tschatzl/8151614/webrev.0_to_1/ (diff) http://cr.openjdk.java.net/~tschatzl/8151614/webrev.1/ (full)

Thanks, Thomas

More information about the hotspot-gc-dev mailing list