RFR (S) 8182397: Race in field updates when creating ArrayKlasses can lead to crash

[Andrew Haley]

On 25/07/17 09:29, Erik Österlund wrote:
> 
> On 2017-07-24 19:40, Andrew Haley wrote:
>> On 24/07/17 17:44, Erik Osterlund wrote:
>>> Sorry for jumping in to the conversation a bit late.
>>> Since David Holmes is on vacation, I have to take his place in
>>> questioning the elision of acquire on the reader side being
>>> okay. Because I know he would if he was here.
>>>
>>> In hotspot we almost *never* elide the acquire on the reader side,
>>> relying on dependent loads.
>>
>> I'm sure we do in plenty of places, it's just undocumented or maybe
>> even unknown to the original author.
> 
> That should be purged from the code. With fire.

Possibly, but let us not burn ourselves in the process.

>> We certainly do this a lot in the code that the JIT compilers
>> generate, as you note below.
> 
> That is more okay when we are in control of the code generation.
> Consume-like semantics have the following levels of support:
> 
> 1) No required support: Machines with strong enough memory ordering to 
> guarantee all normal loads and stores have acquire/release semantics 
> require no support.
> 
> 2) Compiler support: Some machines can rely on dependent loads, but what 
> looks like a dependent load in the C++ source code might not translate 
> to dependent loads in the compiled machine code. In order to enforce 
> that, the data dependency chains from the source code must be retained 
> in the generated machine code.

Yes, yes, we know.  :-)

> As for the JIT-compiled code - it is fine to use consume-like semantics 
> if we control the code generation and know that the generated machine 
> code will respect the data dependencies that we wanted it to. But since 
> we do not control the C++ code generation, and do not have access to 
> tools like C++11 consume semantics, we can not just elide acquire and 
> hope for the best. Instead we have to use acquire until OrderAccess 
> starts supporting consume (if it ever will).

OrderAccess could support consume tomorrow: that's up to us.

> What we perhaps could do is to add shared code to OrderAccess for 
> load_consume, implement them as load_acquire, so that we at least 
> remember where in the code we could have gotten away with dependent 
> loads and exploit it, and later on plug it into C++11 consume so that 
> C++ compilers that support such contracts can utilize it on a platform 
> by platform basis.

I suspect we could make it work immediately, because we know how to
emit compiler fences on all platforms (do we?) and almost all of them
respect data dependencies in hardware anyway.  Those that don't will
have to use acquire.

>> I don't think it's that much of a big deal, really.  Putting in an
>> acquire fence every time we read a java mirror strikes me as madness
>> when consume is all we need on any target.  Even Power PC doesn't need
>> a fence instruction there.  At least make sure that targets can turn
>> that off: we should not pay the price for something we don't need.
> 
> Conversely, those targets would pay the price of compromised
> correctness - a price higher than performance regressions.
> Performance can only be optimized within the legal bounds of
> correctness.

Please, spare us the lecture.  No-one was sugesting compromising
correctness.

> Therefore we can only elide fences if we have well defined memory
> ordering semantics for doing that and somehow control that the
> generated code adheres to those semantics and respects them. We
> simply do not have consume semantics today, and therefore to ensure
> correctness, we must use acquire, until the day comes when we
> support acquire.

I guess you meant "consume" here?  Anyway, all we need is a compiler
fence.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671