Thanks for review, Paul.
Best regards,
Vladimir Ivanov
On 19.11.2019 21:25, Paul Sandoz wrote:
+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.
