RFR: 8133051: Concurrent refinement threads may be activated and deactivated at random

[Kim Barrett]

> On Apr 14, 2016, at 11:21 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>> New webrevs:
>> full: http://cr.openjdk.java.net/~kbarrett/8133051/webrev.01/
>> incr: http://cr.openjdk.java.net/~kbarrett/8133051/webrev.01.inc/

Mikael, thanks for looking at this.

> in concurrentG1RefineThread.hpp
>  81   size_t activation_threshold() const { return _threshold; }
> 
> We usually name accessors to match the member name, but I realize that "threshold()" here is not informative enough.
> How about renaming _threshold => _activation_threshold ?

I've been annoyed by that too, but was minimizing unecessary
differences.  I've finally made the suggested renaming.

> in concurrentG1RefineThread.cpp
> 
> 70 void ConcurrentG1RefineThread::update_thresholds
> 
> has somewhat unusual formatting, the common style (which we agreed upon at some point IIRC) is either

Who's "we"? :-) Seriously, where can that information be found?  For
one thing, I can't lobby for changes to a style guide that doesn't
exist.  Anyway, I changed it.

> I'm not sure that I agree that it's necessary to have the CTRL_TAGS and LOG_ZONES #defines in order to ensure that all three logging call sites use the same set of logging tags.

Having at least twice during development gotten these wrong (there
were more logging sites during some intermediate development stages)
and since I'm expecting to add more logging as part of improving the
control laws, I'm inclined to keep them.

> The rest of the change looks good and it's a lot more understandable what the code is trying to achieve.

Thanks.