RFR: Access barriers for arraycopy

[Roman Kennke]

Ok, I'll wait for green testing then :-)

On Tue, Feb 20, 2018 at 7:29 PM, Aleksey Shipilev <shade at redhat.com> wrote:
> On 02/20/2018 07:19 PM, Roman Kennke wrote:
>>>>> *) Since we are stabilizing sh/jdk*10*, I would prefer not to extend Access API here: when doing
>>>>> this, we are risking unforeseen interactions with the rest of VM and other GCs. Do we actually need
>>>>> the extensions to work correctly?
>>>>
>>>> If we don't want that in 10, they we should get the shenandoah/jdk
>>>> into shape ASAP, because this is work that's important to get
>>>> Shenandoah in shape for upstreaming ;-)
>>>
>>> Yeah, but not before we stabilize sh/jdk10... :/
>>
>> What is the criteria for that?
>
> When the tests are all green. Which they were not at least yesterday, before clone barrier fixes.
> This is why this work confused me into believing it fixes actual sh/jdk10 bug.
>
>> The way I see it is that we could move unstable-ish development to
>> shenandoah/jdk and only backport relevant stabilizing stuff back to
>> shenandoah/jdk10.
>
> Yes. And we are hopefully doing it this week.
>
>>>> It's not an extension of the Access API per se. It#s more like a bug
>>>> fix. There's a call to Raw::arraycopy() that plain simply wrong: that
>>>> call does not accept oop arguments. Since it was never used in
>>>> upstream yet, the compiler never resolved and complains about that. I
>>>> would like to get it into Shenandoah first to give it some exposure
>>>> and testing, then bring it upstream as soon as possible. No, it
>>>> doesn't not work/make sense without those shared code changes.
>>>
>>> OK. So it is still required in sh/jdk10 for correctness.
>>
>> No, not really. The code is correct as it is. Right now we have put
>> the barriers around the callsites to arraycopy/memcpy calling code.
>> This change moves into a proper location in the Access API under GC
>> specific directory. Overall it reduces upstream exposure (assuming the
>> required shared code changes go upstream, which I assume they will).
>
> So here is my line of thinking: sh/jdk10 is the *backport* repository, and it should really avoid
> changes that are not upstream, pretty much like sh/jdk8 and sh/jdk9 do. So, if we want a cleanup
> that reduces upstream exposure -- good, it can go to sh/jdk{8,9,10}! But if that cleanup introduces
> new things in shared code, and we don't know the impact of those (e.g. I am not very sure that
> Access API internals are docile enough to accept small one-liner "obvious" changes, and it does not
> break spectacularly), and we are pushing them to test before upstreaming -- then it is the thing for
> sh/jdk, not for any backport repository.
>
> So, if this does not fix a bug in sh/jdk10, I would hold this off for sh/jdk. Let sh/jdk10 use the
> old and trusted (yet more exposed) code.
>
> -Aleksey
>