RFR: Refactor evac-in-progress flag to more general gc-phase flag

[Aleksey Shipilev]

On 09/14/2017 07:09 PM, Roman Kennke wrote:
> Here's another one in preparation of concurrent partial GC:
> 
> This patch refactors the Thread::evacuation_in_progress flag to a more general gc_phase_in_progress
> flag. Instead of checking for 0 or != 0, it must now be masked with an appropriate gc-phase-mask
> before checking. Each bit represents a GC phase. For now, I've only done EVACUATION on bit 0.
> 
> The idea here is that when storing an oop, we usually do:
> 
> - check satb-in-progress (and do satb-barrier if so)
> - check evacuation-in-progress (and do write-barrier if so)
> 
> and with conc partial, we want to check for conc-partial-in-progress and do a special barrier too.
> 
> Putting all in one flag allows us to load the flag once, and test it repeatedly. The compiler may
> even common this flag-loading over multiple stores.
> 
> Test: hotspot_gc_shenandoah on x86 and aarch64
> 
> http://cr.openjdk.java.net/~rkennke/gc-phase-flag/webrev.00/

*) There is no commonning for SATB and evac flags in this patch, right?

*) aarch64 tests for EVACUATION_BITPOS:

  63   __ tbz(rscratch1, ShenandoahHeap::EVACUATION_BITPOS, done);

 x86 tests for EVACUATION:

 5623   testb(gc_phase_in_progress, ShenandoahHeap::EVACUATION);

 Why this discrepancy?

*) Indent is wrong:

1610   static ByteSize gc_phase_in_progress_offset() { return byte_offset_of(JavaThread,
_gc_phase_in_progress); }

*) I wonder if state getters should really be left as "bool"-s:

 166 char JavaThread::gc_phase_in_progress() const {
 167   return _gc_phase_in_progress;
 168 }

 ...e.g.:

     bool JavaThread::evacuation_in_progress() const ...

 Can't say for sure without seeing where is going.

Thanks,
-Aleksey