Regarding the extra cast in accessor logic that Paul picked up on: That may be a left-over from obsolete versions of the code, or it may cover for some corner cases, or it could possibly be a re-assurance to the JIT.
Generally speaking, we lean heavily on MH types to guarantee a priori correctness of argument types. Paul is right that the stored value type is already validated and (except for corner cases we may be neglecting) does not need re-validation via a cast. It might be an interesting experiment to replace the cast with an assert. Sometimes corner cases bite you. For example, an array store check is necessary, if the type is an interface, because interfaces are weakly checked all the way up to aastore or invokeinterface. Sometimes the JIT cannot see the type assurances implicit in a MH type, and so (when choosing internal MH code shapes) we must choose between handing the JIT code that is not locally verifiable, or adding "reassurance" casts to repair the local verifiability of the code. If the JIT thinks it sees badly-typed code, it might bail out.Note that "locality" of verifiability is a fluid concept, depending sometime on vagaries of inlining decisions. This is the reason for some awkward looking "belt and suspenders" MH internals, such as the free use of casts in LF bytecode rendering. Usually, redundant type verifications (including casts and signatures of incoming arguments) are eliminated, but they can cause (as noted) an extra null check. In theory, that should fold up also, if the null value is replaced by "another" null, as (p == null ? null : identityFunction(p)). — John On Jul 8, 2014, at 3:09 AM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> wrote: > I'd like to revive this review thread. > > Updated version: > http://cr.openjdk.java.net/~vlivanov/8037209/webrev.04/ > > Paul, I'd like to address your case (if possible) separately. Right now, I > don't see any way to avoid null check you see with your tests. On Apr 1, 2014, at 8:29 AM, Paul Sandoz <paul.san...@oracle.com> wrote: > On Apr 1, 2014, at 4:10 PM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> > wrote: > >> Paul, >> >> Unfortunately, additional profiling doesn't work for Accessor.checkCast >> case. The problem is Accessor.checkCast is called from multiple places, so >> type profile is very likely to be polluted. And it kills the benefits. >> > > So is there any point in doing such a cast in this case? > > The type performing the cast is the field type declared as a parameter in the > MethodType of the MethodHandle and also held by the Accessor. > > IIUC the Invokers.checkExactType should ensure no "unsavoury" instances of > the wrong type gets through? (the holder instance is only checked for null, > via checkBase).
