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