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

[Thomas Schatzl]

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::clear_prev_bitmap(WorkGang* workers) {
> 729 assert(SafepointSynchronize::is_at_safepoint(), "Should only
> clear 
> the entire prev bitmap at a safepoint.");
> 730 clear_bitmap((G1CMBitMap*)_prevMarkBitMap, workers, false);
>   731 }
> 
> Would it be appropriate to move the assertion at 729 to
> 
> 698 void G1ConcurrentMark::clear_bitmap(G1CMBitMap* bitmap, WorkGang*
> workers, bool may_yield) { assert(may_yield
> ||SafepointSynchronize::is_at_safepoint(), "Should only clear the
> entire 
> bitmap at a safepoint."
> 
>   699 G1ClearBitMapTask task(bitmap, this, workers->active_workers(),
> may_yield);
> 700 workers->run_task(&task);   701 guarantee(!may_yield ||
> task.is_complete(), "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