Overall, looks really good! I like how it shapes out.

A couple of nitpicks:

(1) Please, use more meaningful names: INJECTED_INVOKER_TEMPLATE vs T_BYTES, invokerTemplateGenerator vs tBytesGenerator, InjectedInvoker vs T.

  (2) There's no need in a constructor. The invoker is never instantiated.

(3) I don't see any point in T.init(). It is called from BindCaller.makeInjectedInvoker [1], but: (1) it's initialized later anyway [2]; and (2) Unsafe.ensureClassInitialized can be used.

Frankly speaking, I'd prefer [2] to be converted into an assert. Also, I don't see any point in forcing the initialization of the invoker class during construction. I think it can go away as well.

Best regards,
Vladimir Ivanov

[1] http://hg.openjdk.java.net/jdk9/dev/jdk/file/698b526d7c3b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java#l1123

[2] http://hg.openjdk.java.net/jdk9/dev/jdk/file/698b526d7c3b/src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java#l1137

On 5/12/16 3:02 PM, shilpi.rast...@oracle.com wrote:
Thank You Paul for comments.
Please review updated webrev
http://cr.openjdk.java.net/~srastogi/8149574/webrev.08/

Regards,
Shilpi

On 5/12/2016 3:41 PM, Paul Sandoz wrote:
On 11 May 2016, at 18:31, shilpi.rast...@oracle.com wrote:

Hi All,

Please review the updated webrev-
http://cr.openjdk.java.net/~srastogi/8149574/webrev.07/


1219             FieldVisitor fv;
1220             MethodVisitor mv;
1221             AnnotationVisitor av0;

Field “fv is not used. Since “av0” is only used once, might as well
declare it at line #1246.

Can you break up the long lines at #1242 & #1252 ?


My inclination is to turn the anon static block into a method
returning byte[] and then do:

/*
<JAVA DOC explaining the class that is generated rather than just
floating as is the current case>
*/
private static final byte[] T_BYTES = generateT();
private static byte[] generateT() {
…
}


One concern, more so because of my ignorance, is why can the default
package be used. Does anyone know?

Paul.

Changed the anonymous class package with no package name.

Regards,
Shilpi


Reply via email to