RFR: 8213890: Implementation of JEP 344: Abortable Mixed Collections for G1

[Kim Barrett]

> On Nov 26, 2018, at 4:03 PM, Stefan Johansson <stefan.johansson at oracle.com> wrote:
> 
> Thanks Thomas,
> 
> Updated webrevs
> Full: http://cr.openjdk.java.net/~sjohanss/8213890/02/
> Inc: http://cr.openjdk.java.net/~sjohanss/8213890/01-02/

Looks good.

Just a few very minor things, which you can do or not.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectionSet.cpp 
 105 void G1CollectionSet::initialize_optional(uint max_length) {

Consider also asserting _option_region_length == 0.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1CollectionSet.cpp 
 451 bool G1CollectionSet::optional_is_full() {

Consider asserting _optional_region_length <= _option_region_max_length.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
  40 G1ParScanThreadState::G1ParScanThreadState(G1CollectedHeap* g1h,
...
  86   _oops_into_optional_regions = NEW_C_HEAP_ARRAY(G1OopStarChunkedList, _num_optional_regions, mtGC);
  87   for (size_t i = 0; i < _num_optional_regions; i++) {
  88     ::new (_oops_into_optional_regions + i) G1OopStarChunkedList();
  89   }

Why not something like

  _oops_in_optional_regions = new G1OopStarChunkedList[_num_optional_regions];

and the corresponding delete[] in ~G1ParScanThreadState().

This would require adding ~G1OopStarChunkedList to free the chunk
lists (possibly eliminating the need for public free_chunk_lists). It
probably also requires giving G1OopStarChunkedList some mechanism for
getting the "used_by_optional" information.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/heapRegion.hpp
 253   // The index in the optional regions array, if this region
 254   // is considered optional during a mixed collections.
 255   uint _index_in_opt_cset;
 256   int  _young_index_in_cset;

It's surprising (and somewhat disturbing) that the type of the new
_index_in_opt_cset different from the type of the pre-existing
_young_index_in_cset. It seems to be correct, but unpleasant. I'm
guessing you get fewer annoying casts in the new code by doing it this
way? (With UINT_MAX as the "undefined" value.) I wonder if
_young_index_in_cset would benefit from similar treatment. Maybe an
RFE?

------------------------------------------------------------------------------