On Fri, 21 Mar 2025 17:26:08 GMT, Quan Anh Mai <qa...@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
