RFR: 8189871: Refactor GC barriers to use declarative semantics

[Erik Österlund]

Hi Kim,

My apologies - I already pushed my changes and did not realize you had 
more feedback.

On 2017-11-20 20:37, Kim Barrett wrote:
>> On Nov 17, 2017, at 9:09 AM, Erik Österlund <erik.osterlund at oracle.com> wrote:
>>> When we discussed this problem previously, we talked about having
>>> specific names associated with categories of off-heap references that
>>> might be handled differently by different collectors.  Some specific
>>> examples that came up in that discussion were JNI global (and weak)
>>> handles, and interned strings.  Even if we really expect all of our
>>> concurrent collectors to eventually process all of these concurrently,
>>> such features might be added on different schedules for different
>>> collectors.
>>>
>>> Without such a naming scheme, e.g. with only a generic
>>> IS_CONCURRENT_ROOT, different collectors may pay the cost for being
>>> conservative for different categories.  That might be acceptable if
>>> being conservative is cheap.  But then I would expect the possibly
>>> concurrent case to be pretty much the default, to be used nearly
>>> everywhere, since it is unreasonable to expect non-GC experts to stay
>>> up to date on which roots are never ever scanned concurrently by any
>>> collector.
>> Okay so I really want the ability to say my roots are definitely scanned in a pause, e.g. when storing oops in thread-local value type values that are considered semantically as part of the stack, and will definitely for all thinkable weird forseeable GC combinations be scanned in a pause.
>>
>> At the moment it is really a special case with potentially concurrently scanned roots rather than the norm. So if that is okay, I would like to defer a decision to make concurrent root the default type of root. At least now that we are annotating the access, if any GC developer is adding concurrent processing of roots, it is very easy to find which where potential trouble lies.
> To me, that suggests there should be an IN_STACK_ROOT decorator.
>
> And I have reason to think the "will definitely for all thinkable
> weird forseeable GC combinations be scanned in a pause" set is empty.
>
> I guess I'm okay with deferring some of the design in this specific
> area for the moment. (I was going to say "defer until needed", but
> with Shenandoah and ZGC that might be essentially now, and even G1
> could probably benefit pretty quickly.)
>
> I was going to suggest removing IN_CONCURRENT_ROOT, since I really
> doubt any sort of generic label like that is the right approach, and
> it's very nearly unused. But it appears there are no uses as actual
> access annotations yet, and the places where it is used appear to be
> parts of the implementation that would need to be adapted for any
> redesign, so maybe it's worth keeping as a placeholder.

Yes I think time will tell best what to do here.

> While looking at uses of IN_CONCURRENT_ROOT, I found a few more things
> to comment on.
>
> In Access::verify_heap_oop_decorators, I think (IN_ROOT ^
> IN_CONCURRENT_ROOT) should be using | rather than ^.

IN_ROOT ^ IN_CONCURRENT_ROOT is equal to IN_ROOT | IN_CONCURRENT_ROOT
But yes, perhaps that would have looked nicer.

> In Access::verify_(primitive|oop|heap_oop)_decorators, I don't
> understand the expected_mo_decorators template parameter. If it is the
> set of allowed memory order decorators, it is being effectively
> ignored by the inclusion of MO_DECORATOR_MASK in the function-specific
> decorators set.
>
> In Access::verify_decorators, all the category checks allow a zero
> value in the category being checked. I wouldn't expect a zero value to
> be permitted; for all categories there's an explicit default, which
> ought to be present. Is this verification done before defaulting?  If
> so, then is there a later check that there's no bug in defaulting that
> would introduce an inconsistency?

The verify_decorators stuff checks the decorators passed in by the users 
only.
For each type of access there is first a check that decorators outside 
of a certain mask are not used (nonsense decorators for a given access 
type).
When we know there are no nonsense decorators, we check for each set of 
decorators that either precisely one or no decorator conbination has 
been selected.
For the sets that did not have a decorator value, defaults are applied. 
For example, memory ordering is by default unordered for loads/stores, 
seq_cst for xchg and cmpxchg, and reference strength is by default 
AS_NORMAL.

So to answer the question: it is fine for decorator sets passed in by 
users to not explicitly set default decorator values, which indeed will 
be set later on.

Thanks,
/Erik