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.

Fair enough.

> 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.

Right, good catch. It should work because normal Shenandoah does not 
evacuate during marking. It may have side effects though because we use 
TAMS elsewhere too. I should probably wrap it in 
if(is_concurrent_traversal_in_progress()) or such.

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

Will check it. Might actually be related to the TAMS issue above.

> 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()) {

No. With this patch, we don't traverse through alloc-regions at all. 
It's kindof a mix: for the sake of traversal we pretend it's all live 
and don't traverse through it (because we don't evac them anyway), but 
then we also don't count liveness in them, so that they will get picked 
into cset in the next cycle (just like between-cycle alloc-regions). In 
order to prevent reclamation of alloc-regions with live stuff in them, I 
need to special case them. Yes, it does preclude immediate reclamation. 
Does that happen a lot for alloc-regions anyway?

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

Same explanation as above.

> *) 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)) {
> 
> *) Comment is obsolete.
> 
>   981           _verify_liveness_disable,    // liveness data must be complete here

Will fix them together with the other stuff. Stay tuned for updated webrev.

Cheers, Roman


-- 
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.