On Sep 13, 2008, at 7:27 AM, Rémi Forax wrote: > currently CallSite.target is not volatile, so if an invokedynamic > bootstrap > method calls setTarget() in one thread, other threads may not see the > new target. > > Proposed changes : > - make target volatile > - use unsafe.putOrdered to set the field in a more lightweight way.
Hmm... The EDR is ambiguous about this, and needs to be tightened up. It does say that the bulk invalidation API is intended to provide a global interlock, and gives some freedom to the propagation of setTarget effects. What the EDR says about "next call" refers to a global ordering that doesn't really exist in the JVM apart from synchronization. Native JVM implementations don't require target to be explicitly volatile. In fact, individual threads need the freedom to cache the target, e.g., as a loop invariant for a fast-path version of a loop. The important thing to optimize here is the speed of *reading* the target field. Writes can be as slow as we want (I use a native method call). All this is independent of whether the code says "volatile" or not on the field, but it may be better not to mark it volatile, since that seems to make a guarantee about propagation of setTarget effects. Note that the JVM reads the target field in a specialized way, not necessarily through a bytecode. Also, the writes to the target field go through an accessor, so once again, the volatility of the field does not necessarily matter. Except to the backport. For the backport, I guess the volatile bit is needed. So, yes, that's a good change. Maybe the right thing to do is move target into a mixin-style superclass, java.dyn.impl.CS, and have it be volatile in the backport. This takes me to the second proposed change. > - change alls field visibility from protected to private because : > 1) protected fields appear in the JavaDoc, > so another implementation of CallSite (like the one currently > use by the backport) > is not allowed. > 2) caller, name and type are final so caller(), name() and type() > can be used instead. > subclasses can override checkTarget() to do nothing, in that > case setTarget() is equivalent to change the target field. > So I see no reason to declare this fields protected. I want the CallSite to be usable two ways: 1. created by the JVM to reify invokedynamic instructions 2. creatable by Java code to simulate invokedynamic sites interpretively Therefore, it's important to allow Java code to make its own flavor of CallSite. Hence the protected methods & fields. This could also be done with a CallSite interface, but it seems easier to me (and to other JVM vendors I think) to make it a concrete class. Therefore, let's keep the protected stuff for now, unless we decide to make CallSite into an interface. I want to factor the 292 API into common classes, including the public ones, and non-standard implementation-specific classes, one version for each kind of JVM and one for the backport. The java.dyn.impl package contains the implementation-specific classes. (This name will change to go outside of the java.* hierarchy.) When a public type needs to mix in fields or methods, it does so via inheritance from an implementation superclass. Switching between backport and native implementations will therefore require adjustments to boot class path or the installed rt.jar. Why use mixins? Although such pluggable implementations are more often done via delegation or subclassing, the performance requirements of method handles requires a more aggressive (less flexible) pattern of single-inheritance mixins. -- John _______________________________________________ mlvm-dev mailing list [email protected] http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
