On Wed, 19 Mar 2025 01:25:24 GMT, John R Rose <jr...@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). We use `@Stable` + `volatile` in a number of places: * https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/Module.java#L273 * https://github.com/openjdk/jdk/blob/master/src/jdk.unsupported/share/classes/sun/misc/Unsafe.java#L1762 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. So, avoiding `volatile` can buy something in the slow paths -- imperative set, maybe for predicates. But how important is that (given stable values are only set once) ? For this reason, at least in my mind, I'd rather opt for an implementation that is easier to follow (even 10 months from now) -- of course, assuming it's fast enough in the fast path (which seems to be the case here) -- than having an uber-optimized implementation whose quirks we'll have to re-learn every time we touch the code. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23972#issuecomment-2743779395
