+1
> On Nov 19, 2019, at 10:12 AM, Vladimir Ivanov <[email protected]>
> 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.
