RFR: Traversal: Don't traverse new objects

[Roman Kennke]

Am 06.02.2018 um 21:46 schrieb Aleksey Shipilev:
> On 02/05/2018 12:16 PM, Roman Kennke wrote:
>> With Traversal GC I'm currently observing final-pauses of several dozen (e.g. ~70) ms. Turns out
>> that almost all of it is just traversing just-allocated objects. It occurs to me that this is
>> pointless work: we're not going to evacuate any new object during this cycle, and liveness
>> information may not be that useful either into next cycle because it usually degenerates quickly
>> (for new stuff). Let's not traverse new-alloc-regions, and don't count liveness there, which means
>> it's going to end up in next cycle's cset (just like the alloc-regions between cycles). This cuts
>> pauses down to 3-4ms. Yay!
>>
>> Notice that this is safe because new objects don't point to anything, and any ref stored into new
>> objects during the pause get picked up by the enqueing barrier for I-U.
>>
>> Notice that we *do* need to traverse and account evac-regions, because they have references pointing
>> to interesting stuff. Hence the disctinction for is_alloc_region() (better ideas welcome!).
>>
>> Also notice that this opens up an interesting opportunity for optimizing the enqueue-barrier: we
>> could now elide ENQBs on objects known to be new. (In addition to known-NULL, which we already
>> check, and already-enqueued, which we don't do yet. We could also do pretty much what we do with WBs
>> and RBs and hoist them out of loops, etc. Lots of work to do there, and opportunities for
>> improvements).
>>
>> http://cr.openjdk.java.net/~rkennke/traversal-no-traverse-new/webrev.00/
>
> I need more time to understand the logic of this patch. It is seriously weird to see next-TAMS setup
> in ShenandoahFreeSet::try_allocate_in, which is on generic allocation path which also should work
> for non-Traversal GC modes.

I guarded the new parts with appropriate if
(conc_traversal_in_progress()) checks..

> The patch fails at least AllocHumongousFragment with:
>   #  assert(!_heap->is_marked_next(oop(humongous_obj))) failed: must not be marked

This one is caused by humongous regions not getting flagged as
alloc_regions during allocation. Fixed in the new changeset.

> Other comments:
>
> *) So wait, this means alloc-regions are not the candidates for immediate reclamation now?
>
>   546         if (r->is_humongous_start() && !r->has_live() && !r->is_alloc_region()) {

Yes, exactly.

> *) Same for regular trashing?
>
>   556         } else if (!r->is_empty() && !r->has_live() && !r->is_alloc_region()) {

Yep.

> *) Excess parentheses:
>
> shenandoahTraversalGC.cpp:
>    73       if ((!_heap->is_marked_next(obj)) && _heap->mark_next(obj)) {
>
> shenandoahTraversalGC.inline.hpp:
>    54     if ((!_heap->is_marked_next(obj)) && _heap->mark_next(obj)) {
>

Fixed.

> *) Comment is obsolete.
>
>   981           _verify_liveness_disable,    // liveness data must be complete here

Fixed.

Differential:
http://cr.openjdk.java.net/~rkennke/traversal-no-traverse-new/webrev.01.diff
Full-patch:
http://cr.openjdk.java.net/~rkennke/traversal-no-traverse-new/webrev.01/

Passes all hotspot_gc_shenandoah tests

Roman