RFR: Refactor to group marking bitmap and TAMS structure in one class ShenandoahMarkingContext

[Roman Kennke]

Am 17.07.2018 um 22:49 schrieb Aleksey Shipilev:
> On 07/17/2018 10:20 PM, Roman Kennke wrote:
>> This is a long overdue refactoring. It groups the two structures
>> involved in marking (bitmap + TAMS) into one class
>> ShenandoahMarkingContext. This allows for some interesting improvements:
>> - Easy to keep in sync (e.g. simpler swapping and reasoning-about)
>> - Can keep actual arrays const
>> - Can pass around the context (see upcoming patch)
>> - Code related to marking and other bookkeeping is now grouped together
>> in one class
>> - Reduce duplications next vs. complete versions of methods
>>
>> The code now makes obvious where we have extra dereferences. I felt the
>> itch to reduce XX_marking_context() calls in a bunch of places, but
>> wanted to keep this a mechanical refactoring as much as possible.
>> Upcoming patch resolves some of the hottest indirections. In any case,
>> it's not worse than it was before (it only looks worse).
>>
>> Testing: tier3_gc_shenandoah ok
>>
>> http://cr.openjdk.java.net/~rkennke/refactor-marking-context/webrev.00/
> 
> Looks cute!
> 
>  *) This method is probably "swap_mark_contexts" now:
>   1994 void ShenandoahHeap::swap_mark_bitmaps() {
> 

Fixed.

>  *) ShIsAlive, ShForwardedIsAlive, ShIsMarkedNextClosure closures are also performance-sensitive...

Fixed for ShIsMarkedNextClosure. The other two have a weird livecycle.
We have two global instances of them that remain alive forever. IIRC
this is mostly for our ReferenceProcessor(s) that also remain alive
forever. We could reset the context for each cycle, but this would
sortof defeat the optimization. We should think hard about fixing the
lifecycles of our is_alive-closures and RefProcs instead.

>  *) context()->mark_bit_map()->clear_range_large(...) can probably be put in utility method, e.g.
> context()->clear_bitmap(...) or something. In fact, mark_bit_map() should only be used when
> something wants the naked bitmap, e.g. Verifier.

Good idea, done that.

>  *) Seems to me, you can use local variables a lot to both simplify the code, and micro-optimize. E.g.:

Yes, done that too, except for the mark loops which will be addressed by
subsequent patch.

Incremental:
http://cr.openjdk.java.net/~rkennke/refactor-marking-context/webrev.01.diff/

Full:
http://cr.openjdk.java.net/~rkennke/refactor-marking-context/webrev.01/

Better?