+1 > On Nov 19, 2019, at 10:12 AM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> > wrote: > > Thanks, Paul. > >> CallSite: >> public class CallSite { >> - // The actual payload of this call site: >> + // The actual payload of this call site. >> + // Can be modified using {@link >> MethodHandleNatives#setCallSiteTargetNormal} or {@link >> MethodHandleNatives#setCallSiteTargetVolatile}. >> /*package-private*/ >> - MethodHandle target; // Note: This field is known to the JVM. Do not >> change. >> + final MethodHandle target; // Note: This field is known to the JVM. >> Is there any benefit to making target final, even though it's not really for >> mutable call sites? (With the recent discussion of "final means final" it >> would be nice to not introduce more special case stomping on final fields if >> we can avoid it). > > CallSite.target is already treated specially: all updates go through > MethodHandleNatives and JIT-compiler treat it as "final" irrespective of the > flags. > > My main interest in marking it final is to enforce proper initialization on > JDK side to make it easier to reason about: > > https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-November/036021.html > >
Ok, I see now in light of that context. >> CallSite(MethodType targetType, MethodHandle createTargetHook) throws >> Throwable { >> - this(targetType); >> + this(targetType); // need to initialize target to make CallSite.type() >> work in createTargetHook >> ConstantCallSite selfCCS = (ConstantCallSite) this; >> MethodHandle boundTarget = (MethodHandle) >> createTargetHook.invokeWithArguments(selfCCS); >> - checkTargetChange(this.target, boundTarget); >> - this.target = boundTarget; >> + setTargetNormal(boundTarget); // ConstantCallSite doesn't publish >> CallSite.target >> + UNSAFE.storeStoreFence(); // barrier between target and isFrozen updates >> } >> I wonder if instead of introducing the store store fence here we could move >> it into ConstantCallSite? > > Sure, if you prefer to see it on ConstantCallSite side, we can move it there. > > By putting it in CallSite near the call site update, I wanted to stress > there's a CallSite.target update happening on partially published instance. > Up to you. Paul.