<div dir="ltr"><br>> Message: 1<br>> Date: Thu, 11 Jul 2024 08:50:57 GMT<br>> From: John R Rose <<a href="mailto:jrose@openjdk.org">jrose@openjdk.org</a>><br>> To: <<a href="mailto:hotspot-dev@openjdk.org">hotspot-dev@openjdk.org</a>><br>> Subject: Re: RFR: 8333791: Fix memory barriers for @Stable fields<br>> Message-ID:<br>>         <pfFWmbs1q_M-WQIDyBw15ctVdRcAudSrdJ6BEQRx41E=.<a href="mailto:762c100f-7650-47fd-bfe3-ac620913384f@github.com">762c100f-7650-47fd-bfe3-ac620913384f@github.com</a>><br>><br>> Content-Type: text/plain; charset=utf-8<br>><br>> On Mon, 10 Jun 2024 18:05:09 GMT, Aleksey Shipilev <<a href="mailto:shade@openjdk.org">shade@openjdk.org</a>> wrote:<br>><br>> > See bug for more discussion.<br>> ><br>> > Currently, C2 puts a `Release` barrier at exit of _every_ method that writes a `@Stable` field. This is a problem for high-performance code that initializes the stable field like this: <a href="https://github.com/openjdk/jdk/blob/79a23017fc7154738c375fbb12a997525c3bf9e7/src/java.base/share/classes/java/lang/Enum.java#L182-L193">https://github.com/openjdk/jdk/blob/79a23017fc7154738c375fbb12a997525c3bf9e7/src/java.base/share/classes/java/lang/Enum.java#L182-L193</a><br>> ><br>> > A more egregious example is here, which means that every `String` constructor actually does `Release` barrier for `@Stable` field write, while only a `StoreStore` for `final` field store would suffice:<br>> > <a href="https://github.com/openjdk/jdk/blob/79a23017fc7154738c375fbb12a997525c3bf9e7/src/java.base/share/classes/java/lang/String.java#L159-L160">https://github.com/openjdk/jdk/blob/79a23017fc7154738c375fbb12a997525c3bf9e7/src/java.base/share/classes/java/lang/String.java#L159-L160</a><br>> ><br>> > AFAICS, the original intent for Release barrier in constructor for stable fields was to match the memory semantics of final fields better. `@Stable` are in some sense "super-finals": they are foldable like static finals or non-static trusted finals, but can be written anywhere. The `@Stable` machinery is intrinsically safe under races: either a compiler sees a component of stable subgraph in initialized state and folds it, or it sees a default value for the component and leaves it alone.<br>> ><br>> > I [performed an audit](<a href="https://bugs.openjdk.org/browse/JDK-8333791?focusedId=14688000&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14688000">https://bugs.openjdk.org/browse/JDK-8333791?focusedId=14688000&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14688000</a>) of current `@Stable` uses for fields that are not currently `final` or `volatile`, and there are cases where we write into `@Stable` fields in constructors. AFAICS, they are covered by final-field-like semantics by accident of having adjacent `final` fields.<br>> ><br>> > Current PR implements Variant 2 from the discussion: makes sure stable fields are as memory-safe as finals, and that's it. I believe this is all-around a good compromise for both mainline and the backports: the performance is improved in one the path that matter, and we still have some safety margin in face of accidental removals of adjacent `final`-s, or in case I missed some spots during the audit.<br>> ><br>> > C1 did not do anything special for `@Stable` fields at all, fixed those to match C2. Both Zero and template interpreters for non-TSO arches put barriers at every `return` (with notable exception of [ARM32](<a href="https://bugs.openjdk.org/browse/JDK-8333957)">https://bugs.openjdk.org/browse/JDK-8333957)</a>), which handles everything in an overkill manner.<br>> ><br>> > Additional testing:<br>> >  - [x] New IR tests<br>> >  - [x] Linux x86_64 server fastdebug, `all`<br>> >  - [x] Linux AArch64 server fastdebug, `all`<br>><br>> I like this compromise.  Let me see if I got it right:  A stable write in a constructor is treated like a final write ? it triggers a barrier at the end of the constructor.  That?s a cheap move.  No other barriers are added automatically, for reads or other writes, saving us from doing less cheap moves.  The burden would be on users of stable vars (in fancy access patterns) to add more fences if needed, but we don?t see any important cases of that, at the moment.<br>><div><br><div>No opinion on the merits here. But IIUC, "as memory safe as finals" is a slightly squishy notion here. The downside of not having the release fence is that even with safe publication, a write to an @Stable field outside the constructor can be seen by a read in the constructor, before the object is published. That's arguably weirder than final field behavior, and not something that can arise with final fields. But it still only happens in the presence of data races, and thus probably not in code you should be writing anyway.</div></div></div>