RFR (L): 8060025: Object copy time regressions after JDK-8031323 and JDK-8057536

[Kim Barrett]

On Dec 17, 2014, at 2:19 PM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Otherwise I would like to separate this naming discussion from this CR, as I would prefer to have this change
> in asap. A few other changes depend on it.

I’m OK with that. Except for the naming question, these changes look good to me.

>> Regarding is_not_in_cset, what exactly does that state mean?  Specifically, does it mean the region
>> is presently unused / inactive?  Or could it be used for something other than young/old/humongous?
>> If the former, then perhaps rename that state (and the predicate) accordingly, e.g. Inactive or something
>> like that.
> 
> The names of the predicates directly derive from the state names.
> 
> A region that has NotInCSet state is not inactive in any way, it's just a region that we do not consider
> for potential collection/reclamation.

> 
> The difference between humongous (candidate) regions (state Humongous) and Young/Old region is only that
> the former is only checked for references, but not evacuated in any way.
> 
> Young/Old regions in the collection set are evacuation sources.
> 
> One option would be to define a collection set that includes both humongous and young/old regions in the
> collection set and the young/old regions into some sort of "evacuation set" (in quotes because I do not like
> the name; typically I would assume that collection set == evacuation set).

How about NotInCSet => OutsideCSet?  OutOfCSet?  That still conflicts with “collection set == evacuation set” for humongous though.  But see above about deferred naming discussion, and below about alternative names for the predicate at issue.

> Then we could have the following predicates:
> 
> static bool is_in_cset()  const { return _value != NotInCSet; }
> static bool is_not_in_cset() const { return !is_in_cset(); } // I guess that this is not true is your point
> of contention

Correct.

> static bool is_in_evac_set() const { return _value > NotInCSet; }
> 
> instead of
> 
> bool is_not_in_cset() const          { return _value == NotInCSet; }
> bool is_in_cset_or_humongous() const { return _value != NotInCSet; }
> bool is_in_cset() const              { return _value > NotInCSet; }
> 
> Alternatively maybe just renaming
> 
>  is_not_in_cset
> 
> above to
> 
>  is_not_in_cset_or_humongous()
> 
> may be as good or better, but is a bit on the long side. It would be the
> negation of is_in_cset_or_humongous() below then (and could be expressed
> as such).
> 
> Or we could just remove the is_not_in_cset() predicate and use !
> is_in_cset_or_humongous() everywhere it's needed.

Any of these seem OK to me.  The last seems like the simplest.

>> Unfortunately, I think the in_cset_state_t typedef needs to be public.  Otherwise,
>> how can a client declare the type of a variable that will hold the result of the
>> value() function.
> 
> It does compile though. I will fix it.

Does that mean there aren’t any callers of value()?  Or does it mean they are making assumptions
about the type it returns, possibly with implicit conversions involved?