RFR: 8351565: Implement JEP 502: Stable Values (Preview) [v6]

[Andrew Dinn]

On Fri, 21 Mar 2025 17:26:08 GMT, Quan Anh Mai <qamai at openjdk.org> wrote:

>>> Hi again Per!
>>> 
>>> Here are some brief notes from our face-to-face chat at JavaOne.
>>> 
>>> Debuggers want/need a "hook" for tentative evaluation of stables. It is an error for a debugger to trigger stable value decisions. This applies mainly to stable lists because of `toString`.
>>> 
>>> Just how "mutable" is a stable list? How "eager to decide"? Which methods (if any) are tentative: `toString` / `equals` / `hashCode` ? Currently in the PR, all are decisive. This might be a case of the “wrong default”.
>> 
>> IMHO if we claim that what the API constructs is a `List`, it would be weird for these methods to behave any different.
>> 
>>> 
>>> Maybe refactor composites to expose systematically "tenative" access API:
>>> 
>>>     * Less universal: SV.list(My::compute) => List
>>> 
>>>     * More universal; SV.stableList(My::compute) => List<StableSupplier>
>>> 
>>> 
>>> BTW, it’s easy to understand a stable-list as a list of stables. But let’s be sure to leave room for a more compact data structure. A compact stable-list is a list of stable views into a backing array. The backing array looks like `@Stable private T[] resolvedValues`. Not `private final List<StableValue<T>> stableValues`.
>> 
>> I don't disagree -- there's two list factories, one that returns a plain List<T> and one that returns a List<SV<T>>. The important thing is we leave room for both (which means naming is important). But I think we're ok for this round even if we don't provide the second factory given that it can be "emulated" (although in a not as compact fashion -- but is that a problem?)
>> 
>> In other words, I'd like to base some of these decisions more on concrete use cases. We had plenty use cases for List<T> -- very few for List<SV<T>>. Maybe some real world use will show that, indeed a List<SV<T>> factory belongs here -- in which case, sure let's add one.
>> 
>> tl;dr; let's get the naming right now -- but add the API later.
>> 
>>> 
>>> For the record: I think this is sufficient for correctness: Use `getAcquire` (resp. `releaseSet`) for all stable reads (resp. writes. Do the `releaseSet` inside a mutex that serializes computation. Add a re-entrancy check in the mutex and throw on vicious cycles.
>>> 
>>> I do NOT think `volatile` is necessary; it has too many fences. It is a safe default for a naked variable. But the stable variables are encapsulated, and do not need aggressive fences.
>> 
>> As I said, `volatile` might not be necessary but it does make the implementation easier to validate (I think)...
>
>> In all these cases there is a fast path: e.g. when we know we have already warned for enable native access, or for Unsafe. In the SV API, the fast path is when we know that the SV is set already. In my experience, the volatile access in this fast path costs nothing: whenever I looked at the generated C2 code for hot paths of FFM code using enable-native-access, it seems that, once the stable field is set, the fact that it is volatile no longer matters. There's no barrier generated by C2 -- access is as fast as plain access.
> 
> An acquire load is allowed to be reordered with a preceding volatile store and I believe this is the only case where it makes a difference. E.g::
> 
>     x = load_acquire(p);
>     store_volatile(p, v);
>     y = load_acquire(p);
> 
> can be transformed into:
> 
>     x = load_acquire(p);
>     y = x;
>     store_volatile(p, v);
> 
> Furthermore, on Aarch64, volatile load is implemented with `ldar` while acquire load can be implemented with `ldr`.

@merykitty 
> Furthermore, on Aarch64, volatile load is implemented with ldar while acquire load can be implemented with ldr.

I'm not sure exactly what you mean here but I don't think that sounds right?

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

PR Comment: https://git.openjdk.org/jdk/pull/23972#issuecomment-2744379545