[lworld] RFR: 8351569: [lworld] Revisit atomic access modes in flat var handles

Thu Mar 20 10:55:19 UTC 2025

On Wed, 19 Mar 2025 18:31:27 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> This PR is an attempt to put var handle support for flat values on a more solid footing. Some more notes in a comment below.

#### Problem

The VM has [several](https://github.com/openjdk/valhalla/blob/lworld/src/hotspot/share/oops/layoutKind.hpp#L28) different flat layouts to pick from. Some layouts support atomic load/stores (but require the value payload, including the null channel -- where present -- to fit in 64 bits) -- we call these layouts *atomic layouts*. Others do not.

When value fields/array elements are accessed through the VarHandle API, a question arises: under which conditions can the API support the various access modes provided by the API? Should all access modes be available, regardless of the JVM layout? Or should non-plain access modes only be supported by atomic JVM layouts?

The current implementation attempts to do the former -- all access modes are supported all the time. This is done by surrounding atomic operations such as CAS with a `synchronized` block. While this strategy is good for prototyping, it is not efficient, and only works correctly _as long as all the accesses_ occur via the `synchronized `block. For instance, if CAS code is mixed with plain access, the CAS code might observe a torn read -- which seems bad.

#### Solution

The VarHandle API should only provide access to non-plain access mode if either a layout is non-flat, or if the layout is flat and *atomic*. In the former case, we can implement access using `Unsafe`'s reference access primitives. In the latter case we can implement access using a combination of `Unsafe::getFlatValue`, memory barriers (for non-plain access modes such as acquire, or volatile) and weak CAS loops. In other words, by limiting support to atomic flat layouts, no `synchronized` block is ever required.

One important design consideration is that we want the set of access modes supported by a `VarHandle` to be *stable* and *predictable*. That is, given that there are many factors influencing which JVM layout is picked for a field or an array element, how do user knows if a CAS operation is going to fail or not?

To provide for more stability I've opted for a simple rule: a field var handle supports atomic operations if either

* it is declared `volatile`
* its type is nullable
* the field type is not a `@WeaklyConsistent` class

For arrays, things are more convoluted: as arrays are covariant, it is always possible to create an array for `Object[].class` but then use it against a flat array. In such cases some dynamic dispatch is unavoidable: the array var handle code has to detect covariant cases, and switch to a "sharper" var handle that models the accessed array. An accessed array instance supports atomic operations if either:

* `ValueClass::isAtomicArray` returns true for the accessed array
* `ValueClass::isNullRestrictedArray` returns false for the accessed array
* the component type of the accessed array is not a `@WeaklyConsistent` class

(Note the above rules mean that var handles targeting migrated value class in [JEP 401](https://openjdk.org/jeps/401) will, by definition, support all access modes -- as all uses of such classes will be through nullable types).

#### Changes

In the code this PR replaces, there are two `VarHandle` implementation classes, namely `VarHandleReferences` and `VarHandleFlatValues`. These classes support _all_ access modes -- one is used for non-flat values, the other is used for flat values.

This first change is to refine the implementation classes so that we can support **four** cases:
* flat values that support non-plain access modes (`VarHandleFlatValues`)
* flat values that do not support non-plain access modes (`VarHandleNonAtomicFlatValues`)
* non-flat values that support non-plain access modes (`VarHandleReferences`)
* non-flat values that do not support non-plain access modes (`VarHandleNonAtomicReferences`)

The `VarHandles::makeFieldVarHandle/makeStaticFieldVarHandle` is then updated so that the correct var handle is returned, based on the characteristics of the field. As mentioned above, for `VarHandles::makeArrayElementHandle`  our strategy is to _always_ create an indirect var handle that supports all access modes (`VarHandleReferences`) -- and then delegate the check at runtime (once we know the array we are operating on). The logic to select a sharp var handle based on the dynamic array instance is in `VarHandles::flatArrayElementHandleFor`.

The decision of whether a given field or array should support non-plain operation is defined in the `VarHandles::isAtomicFlat`predicates (there's one for fields and one for arrays). The logic there follows what discussed above, with one exception: we only treat as atomic value types that have no object pointers in them. This is due to a technical limitation in the `Unsafe` support for the atomic operations (like CAS), where we can turn a flat value CAS into a numeric CAS only as long as the value payload doesn't contain objects (this restrictions prevents us from accidentally updating an object pointer without notifying the GC). While this restriction shouldn't be viewed as permanent, supporting CAS operations on flat values with pointers is a separate challenge that will not be addressed by this PR.

This PR updates most of the non-plain access primitives for flat values in the `Unsafe` class. Atomic operations are now implemented using weak CAS loops. We needed to perform some heroics to turn a value payload into a numeric value (a `byte`, a `short` an `int` or a `long`), so that we can turn a flat value CAS into a numeric CAS -- this logic is in `Unsafe::compareAndSetFlatValueAsBytes`. Non-plain, non-atomic access (e.g. opaque, acquire/release, volatile) are implemented in terms of plain access, with extra barriers -- as described in this [document](https://gee.cs.oswego.edu/dl/html/j9mm.html). This allows us to implement all the access modes on atomic flat layouts in Java. It is possible that, in the future, some (or all) of these methods will turn into JVM intrinsics which will allow to avoid some of the workarounds described here.

Finally, note that even some atomic operations involving reference types have been updated as well -- when a value type is passed to a reference CAS, we need to perform CAS in a way so that it uses the correct definition of `==` (e.g. not pointer comparison). Before this PR this was done, again, with a monitor. All monitor code is now gone.

#### Testing

The main new test added here is `FlatVarHandleTest` -- this checks that field and array element var handles support the expected access modes. This test doesn't focus on whether the _implementation_ of the access primitive is correct -- only on whether the access mode is supported or not. An existing test (`NullRestrictedArraysTest`) has been augmented to test the correctness of the various non-plain mode accesses.

Some test cases in one JVM test (`TestIntrinsics`) had to be disabled: this test expects calls to atomic operations on flat values to always work -- regardless of whether such flat values feature an atomic layout or not. This is incorrect, as the new `Unsafe` primitives only work with atomic flat layouts (as we need to be able to turn a value layout into a numeric value).

src/java.base/share/classes/java/lang/invoke/VarHandles.java line 77:

> 75:                                     : new VarHandleReferences.FieldInstanceReadWrite(refc, foffset, type, f.getCheckedFieldType()));
> 76:                         } else {
> 77:                             return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields

While this case is not terribly important (typically, if something is non-flat, then it will be "atomic" from a programming model perspective), there are various JVM flags that can affect whether something is flattened or not. It's important here that we don't return a var handles that "overpromises" and support more access modes (just because we know that, underneath, we can use reference `Unsafe` primitives) as such behavior might not be stable,

-------------

PR Comment: https://git.openjdk.org/valhalla/pull/1402#issuecomment-2737711409
PR Review Comment: https://git.openjdk.org/valhalla/pull/1402#discussion_r2005271860