RFR: Fold Partial GC into Traversal GC

[Roman Kennke]

Am 26.04.2018 um 10:41 schrieb Aleksey Shipilev:
> On 04/25/2018 11:08 PM, Roman Kennke wrote:
>>> *) Is the move of ShenandoahHeapRegionSetIterator::ShenandoahHeapRegionSetIterator from .inline.hpp
>>> to .cpp sensible? This desyncs backports...
>>
>> Want me to extract this, and push separately? It did not build on 'my'
>> aarch64 box, due to missing inline (tried to compile one definition per
>> call-site, clashed in linking).
> 
> Yes, please push this under the separate changeset, so it can go to backports.
> 
>> Differential:
>> http://cr.openjdk.java.net/~rkennke/partial-traversal/webrev.04.diff/
>> Full:
>> http://cr.openjdk.java.net/~rkennke/partial-traversal/webrev.04/
>>
>> Good now?
> 
> Looks good.
> 
> I have another question, in ShenandoahTraversalGC::process_oop:
> 
>   43     // We need to check is_in_reserved() because this may be called by Metadata traversal.
>   44     bool update_matrix = _heap->is_in_reserved(p);
>   ...
>   67     if (UPDATE_MATRIX && update_matrix) {
>   68       // Successful update of reference. Also update matrix.
>   69       shenandoah_assert_not_forwarded_except(p, obj, _heap->cancelled_concgc());
>   70       _matrix->set_connected(p, obj);
>   71     }
> 
> How's that is_in_reserved correct? It will try to connect the off-heap root to in-heap oop, and then
> we do the store somewhere off the matrix. I guess that never happens during normal cycle, because
> roots would not contain forwarded objects, but it is plausible on DEGEN path? Should be is_in()?
> 

The issue here is that when unloading-classes is on, it traverses
through the metadata of each object, in which case we'd get off-heap ->
on-heap refs. The is_in_reserved() checks if p is off-heap, and if so,
don't record the connection in the matrix.

The better solution would be to do this. Consider the following:

p -> metadata -> obj

What we really want to do here is record the connection p -> obj. I
think this would enable to do do class-unloading in all partial cycles,
not only in the full-traversal cycles. That is why I put the
!is_minor_gc() here:

  virtual bool should_unload_classes() {
    return ShenandoahUnloadClassesFrequency != 0 && ! is_minor_gc();
  }

But that seemed to complex to also add here, and it is not a regression
(we never unloaded classes during partial cycles).

Alas, overnight testing run revealed more crashes/verifier errors that I
want to address before pushing this. Stay tuned...

Roman