RFR: Shenandoah Partial GC with generational/LRU policies

[Roman Kennke]

Am 24.07.2017 um 20:45 schrieb Christine Flood:
> http://cr.openjdk.java.net/~chf/GenerationalPartialGC/webrev.01/
>
> I've made the changes Shade requested and passed all of the tests,
> including the suggested additions, with the exception
> of gc/shenandoah/ShenandoahJNICritical.java which is also broken in a clean
> build.
>
> Christine

- ShenandoahHeuristics::_bytes_allocated_after_last_gc is unused now.
Remove?
- I would find it nicer to make the compare_by_* private static members
of ShenandoahHeuristics.
- I wonder if the is_in() check should be in the normal add_region()
path. After all it's called *Set, I don't think we ever want duplicates,
and is_in() is cheap now. ?
- I don't really like all the timestamp stuff in ShenandoahHeap. Sounds
like it should be in ShenandoahHeuristics or appropriate subclass? Also,
is_conc_gc_in_progress() should be easier to do using the existing
*_in_progress() accessors somehow? E.g.:

bool is_conc_gc_in_progress() {

  return concurrent_mark_in_progress() || is_evacuation_in_progress() ||
is_update_refs_in_progress();

}

- ShenandoahHeuristics::maybe_add_heap_region() ... should probably be a
member of ShenandoahCollectionSet? And dunno if we can come up with a
slightly more descriptive name? Can't quickly think of one though.

- PartialHeuristics::from_idxs should be PartialHeuristics::_from_idxs

1376 void
ShenandoahCollectorPolicy::choose_collection_set(ShenandoahCollectionSet*
collection_set,
1377 bool minor) {
1378 if (minor)
1379 _minor_heuristics->choose_collection_set(collection_set, minor);
1380 else
1381 _heuristics->choose_collection_set(collection_set, minor);
1382 }

There's no need to pass down 'minor', is there? It's not used downstream anyway.

-
59     } else if (heap->shenandoahPolicy()->should_start_partial_gc()) {
60 if (! heap->need_update_refs())
61         service_partial_cycle();

You added a check for need_update_refs() here. My idea is that heuristics that do partial cycles should never need update refs in between. Is this possible now?

There's a whitespace changes that's not for the better IMO:
49 void do_oop_work(T* p) {_partial_gc->process_oop<T, false>(p,
_thread, _queue);} That's all for now :-)