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

[Stefan Johansson]

Thanks Thomas,

Updated webrevs
Full: http://cr.openjdk.java.net/~sjohanss/8213890/02/
Inc: http://cr.openjdk.java.net/~sjohanss/8213890/01-02/

On 2018-11-26 12:03, Thomas Schatzl wrote:
> Hi Stefan,
> 
> On Fri, 2018-11-23 at 15:57 +0100, Stefan Johansson wrote:
>> Thanks Thomas for your review,
>>
> [...]
>>
>>>    - g1CollectionSet.cpp:508: the new code is lacking the message
>>> that if the predicted time of the minimum set of regions we are
>>> going to take already exceeds the pause time goal or not (i.e. the
>>> "expensive regions" message).
>>>
>>> Note that the original message was confusing, see
>>> https://bugs.openjdk.java.net/browse/JDK-8165849; maybe this could
>>> be
>>> fixed here while we are here. Note that I do not see this as
>>> mandatory,
>>> but I would like to have information about that we took additional
>>> regions that will likely exceed the pause time goal.
>>
>> I added another log_debug-line if we have expensive regions, this
>> should make it clear and we keep the information. And yes, after this
>> that issue can be closed.
> 
> You can add multiple CRs in a single commit message to get resolved.
> 

I'm a bit hesitant about this since AMGC is a rather big in comparison 
to the other change, might be confusing if people look at what went into 
the change.

>> [...]
>>
> 
> Some additional minor nits:
> 
> g1CollectionSet.cpp:566; I think the prefix "To ensure progress the" of
> the log message is not necessary.
> 
> g1CollectionSet.cpp:567: s/even if/although (I would think, no native
> speaker)
> 
> g1GCPhaseTimes.hpp:102: unaligned closing curly brace
> 
> g1Policy.hpp:405/408: I would prefer that the description does not
> mention actual values from the code but uses "that percent(age)"
> instead to remove redundancy.

Good comments fixed all of them, but rewrote the last comments a bit more.

Thanks,
Stefan

> 
> Thanks,
>    Thomas
>