Thanks for the examples.
A few comments:
- A reasonable place to consider putting the bootstrap is in Proxy itself. I
am not sure that ConstantBootstraps is the right place (or anywhere in JLI for
that matter) unless Method is retrofitted to implement Constable, which is
problematic given that Methods are not constants.
- In the department of “future things that might be applied here” (Remi
started it), this is a perfect application for lazy statics. But we don’t have
those yet.
- The main benefit here is that it makes proxy creation faster. However,
given the amount of work that goes on at proxy creation (reflection, byte code
spinning, class loading), its not clear how much of the cost is initializing
the proxy class. Some benchmarking to establish the cost benefit here would be
good. (If we are spinning the proxy at jlink time, then the benefit is
probably greater.)
Overall, the translation approach seems sound but I would suggest that we
clarify the benefits a little bit before embarking.
> On Nov 22, 2019, at 4:44 PM, Johannes Kuhn <i...@j-kuhn.de> wrote:
>
> Last mail got converted from HTML into plaintext, which dropped some
> significant whit espace. So, again, doing it by hand this time.
>
> On 22.11.2019 20:52, Brian Goetz wrote:
> > For those that are not intimately familiar with the mechanics of proxy
> > class generation, it would be great to flesh this out with a code example,
> > that shows the proposed old and new translation. This would make it easier
> > for folks to evaluate the benefits and costs of the approach, and possibly
> > suggest improvements.
> >
> > On 11/21/2019 10:23 PM, Johannes Kuhn wrote:
> >>
> >> * What benefits would such a change have?
> >> First, it allows us to drop the static final fields and the class
> >> initializer for the generated proxy classes.
> >> Second, it allows the code (both generated and generator) to be more clear
> >> by moving the essential parts together.
> >> Third, it might bring some performance improvement, because the
> >> j.l.r.Method lookup is delayed to it's first use - which in some cases
> >> might never happen.
> >
> Thanks for the suggestion. A proxy can be used to create an implementation of
> one or more interfaces at run time.
> I just let Java 13 dump a generated Proxy and decompiled it using JD. This is
> the result:
>
> package com.sun.proxy;
> // imports ...;
> public final class $Proxy0 extends Proxy implements Runnable {
> private static Method m1;
> private static Method m3;
> private static Method m2;
> private static Method m0;
>
> public $Proxy0(InvocationHandler paramInvocationHandler) {
> super(paramInvocationHandler); }
>
> public final boolean equals(Object paramObject) {
> try {
> return ((Boolean)this.h.invoke(this, m1, new Object[] { paramObject
> })).booleanValue();
> } catch (Error|RuntimeException error) {
> throw error;
> } catch (Throwable throwable) {
> throw new UndeclaredThrowableException(throwable);
> }
> }
>
> public final void run() {
> try {
> this.h.invoke(this, m3, null);
> return;
> } catch (Error|RuntimeException error) {
> throw error;
> } catch (Throwable throwable) {
> throw new UndeclaredThrowableException(throwable);
> }
> }
>
> public final String toString() {
> try {
> return (String)this.h.invoke(this, m2, null);
> } catch (Error|RuntimeException error) {
> throw error;
> } catch (Throwable throwable) {
> throw new UndeclaredThrowableException(throwable);
> }
> }
>
> public final int hashCode() {
> try {
> return ((Integer)this.h.invoke(this, m0, null)).intValue();
> } catch (Error|RuntimeException error) {
> throw error;
> } catch (Throwable throwable) {
> throw new UndeclaredThrowableException(throwable);
> }
> }
>
> static {
> try {
> m1 = Class.forName("java.lang.Object").getMethod("equals", new Class[]
> { Class.forName("java.lang.Object") });
> m3 = Class.forName("java.lang.Runnable").getMethod("run", new Class[0]);
> m2 = Class.forName("java.lang.Object").getMethod("toString", new
> Class[0]);
> m0 = Class.forName("java.lang.Object").getMethod("hashCode", new
> Class[0]);
> return;
> } catch (NoSuchMethodException noSuchMethodException) {
> throw new NoSuchMethodError(noSuchMethodException.getMessage());
> } catch (ClassNotFoundException classNotFoundException) {
> throw new NoClassDefFoundError(classNotFoundException.getMessage());
> }
> }
> }
>
> The new implementation using ASM generates a similar class.
> I intend to replace the the references to m0, m1... in the generated code
> through an LDC instruction, and remove the code that generates the fields and
> the static class initializer.
>
> The resulting code would look a something like this:
>
> package com.sun.proxy;
> // imports ...;
> public final class $Proxy0 extends Proxy implements Runnable {
>
> public $Proxy0(InvocationHandler paramInvocationHandler) {
> super(paramInvocationHandler); }
>
> public final boolean equals(Object paramObject) {
> try {
> return ((Boolean)this.h.invoke(this, __LDC(__MethodHandle
> (com.sun.proxy.$Proxy0::$bootstap, "_", Method.class,
> __MethodHandle(java.lang.Object::equals)), new Object[] { paramObject
> })).booleanValue();
> } catch (Error|RuntimeException error) {
> throw error;
> } catch (Throwable throwable) {
> throw new UndeclaredThrowableException(throwable);
> }
> }
>
> // repeat for run(), hashCode() and equals(Object other)
>
> private static final Method $bootstrap(Lookup lookup, String ignored,
> Class<?> clazz, MethodHandle mh) {
> return lookup.revealDirect(mh).reflectAs(clazz, lookup);
> }
>
> }
>
> I used __LDC as placeholder for the LDC bytecode instruction (limited to
> constant dynamic), and __MethodHandle for a MethodHandle in the constant pool.
> This means the Method is only looked up when the proxy method is called for
> the first time.
>
> In the actual source code (the generating code), it means that the following
> methods can be dropped:
> ProxyGenerator.generateStaticInitializer()
> https://hg.openjdk.java.net/jdk/jdk/file/a2a921609481/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java#l576
> ProxyGenerator.ProxyMethod.codeFieldInitialization(MethodVisitor, String)
> https://hg.openjdk.java.net/jdk/jdk/file/a2a921609481/src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java#l576
>
> It will require the addition of a new method, which creates the $bootstrap
> method, and a change ProxyGenerator.ProxyMethod.generateMethod to emit an LDC
> instruction instead of a GETSTATIC.
>
> The next step is to remove the now unused ProxyMethod.methodFieldName and
> it's dependencies (constructor, their calls...), but this should be a fairly
> natural change.
> After all those changes, I assume that I will delete more code than I will
> add and change.
>
> Also, as suggested in my last mail, the bootstrap method doesn't need to be
> in the generated proxy class, it just needs to be accessible from the
> generated proxy classes (which can be in any module).
> Remi Forax suggested to put it in the java.lang.invoke.ConstantBootstraps in
> his initial proposal[1] - but this means it would be a public API.
> Using a private method avoids that, at some low cost, and changing to an
> external bootstrap method later would be a small change.
>
> [1]:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-August/061923.html
>
