Record classfile and runtime API

[Remi Forax]

sorry wrong list.

Rémi

----- Mail original -----
> De: "Remi Forax" <forax at univ-mlv.fr>
> À: "valhalla-spec-experts" <valhalla-spec-experts at openjdk.java.net>
> Envoyé: Dimanche 1 Septembre 2019 17:27:13
> Objet: Record classfile and runtime API

> Hi all,
> i think the current generated classfile bytecodes can be improved.
> 
> For a record Point(int x, int y) { }
> 
> The classfile contains:
>  - a specific class attribute Record
>    Record:
>     int x;
>       descriptor: I
>     int y;
>       descriptor: I
>    which allows compiler separate compilation.
>  - two bootstrap methods
>    - one for the invodynamic calls used inside toString/equals/hashCode
>      this bootstrap method takes 4 arguments
>        #8 fr/umlv/record/Point
>        #51 x;y
>        #53 REF_getField fr/umlv/record/Point.x:I
>        #54 REF_getField fr/umlv/record/Point.y:I
>    - one for the constant dynamic used inside the pattern extractor/handler
>      this bootstrap method takes 3 arguments
>        #8 fr/umlv/record/Point
>        #53 REF_getField fr/umlv/record/Point.x:I
>        #54 REF_getField fr/umlv/record/Point.y:I
> 
> so we have 3 different ways to represent exactly the same thing, a record Point
> is composed of two component int x and int y, i believe we can do better.
> 
> The arguments of the bootstrap methods are redundant, you don't need to pass the
> record class because the Lookup object contains the record class (you have the
> API in Lookup to create an intermediary lookup from a Lookup and a class so no
> need to duplicate that information). For the first boostrap methods, the
> parameter containing "x;y" is due to the fact that a constant method handle is
> opaque by default so you can not get the corresponding field name. First, if
> you have the lookup that have created that object, you can get back the name
> with a "reveal". But i think here, the problem is that the arguments are the
> constant method handles and not the constant decriptor (ConstantDec).
> 
> The java.lang.invoke API has currently the limitation that you can not use the
> ConstantDesc API to type values of the constant pool.
> 
> How can we do better ?
> We only need the attribute Record, all other informations can be retrieved at
> runtime from the attribute Record.
> 
> Now we have to talk about the reflection API, currently the method that reflects
> the attribute Record is Class.getRecordAccessors() that returns an array of
> Method corresponding to the accessors and there are many things wrong with this
> method.
> First, while returning the accessors may be a good idea, it should not be the
> sole API, we should have an API that reflects the data of the attribute Record
> itself. Then it returns Method so a live object that may have triggered the
> classloading of the types of each record component that may notexist at
> runtime, we have introduce the constable API, we should using it here. And the
> implementation has several issues, first it can throws a NoSuchMethodException
> which is a checked exception, then it uses getDeclaredMethod which returns the
> methods with the most precise return type which is not what you want here.
> Here is an example that show why it's buggy, le suppose i have an interface and
> a record
>  interface I {
>    default Object x() { return 0; }
>  }
>  record R(Object x) implements I { }
> now let say i change I without recompiling R
>  interface I {
>    default String x() { return 0; }
>  }
> R.class.getRecordAccessors() will return the method x() that returns a String
> instead of the one that returns an Object.
> 
> So first, let's have a method getRecordComponentFields() in java.lang.Class that
> returns an array of DirectMethodHandleDesc corresponding to method ref to the
> fields of a Record. It uses a constant desc, so no weird classloading will
> appear here. And it's better than an array of MethodHandle because a
> DirectMethodHandleDesc is not an opaque object.
> 
> With that, we can unify the bootstrap methods to have only one bootstrap method
> with zero argument, the one that return a pattern/extractor can be implemented
> into another class but can be queried from the same bsm that the one that is
> needed for toString/equals/hashCode.
> 
> BTW, why do we need an extractor for a record ? we should not need one because
> we can inside the algorithm that create the pattern tree have a if
> class.isRecord() ? (the same way the serailization is specilized for enums by
> example).
> 
> On problem with this approach is that while it reduces the bytecode size, it
> will also resolve the constant method handles to the accessors several times
> but i think it's better to have this kind of cache inside java.lang.Class than
> to introduce a constant dynamic that will return a constant list of constant
> method handles instead. Introducing something in the constant pool for caching
> reason seems not a good idea if in the future we want to change the way it
> works.
> 
> Rémi