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.

Reply via email to