RFR: JEP 359-Records: reflection code

[Remi Forax]

The class ObjectMethods is in the package java.lang.invoke if you take a look to the source but in the package java.lang.runtime if you trust the webrev.

It should be in java.lang.invoke so Claes (or someone else) will be able to optimize it later, otherwise if we want to use an internal non public MethodHandle,
we will have to rely on a special SharedSecret class.

I really dislike the bootstrap protocol:
- first it means that you can not have more than 250 fields
- the same call works for equals/hashCode and toString, so depending on the methods some arguments are not used or not
  (names is used only in toString).
- theClass and lookup.lookupClass are the same if used from invokedynamic.
- the whole "if the methodType is null, then return a MethodHandle instead of the callsite" is a hack that leak to the public interface.
- the format 'names' is not the same as the one in the StringConcatFactory and what if the name are not the name of the record components or in wrong order.

I propose the following design to fix these issues:
All invokedynamic should takes the same constant dynamic as parameters typed as an Object.
This object should be an instance of an internal class of ObjectMethods (let say RecordMirror) created or populated by the VM from the Record attribute, from inside the constant dynamic BSM, so there will be only one upcall and one downcall instead of one upcall per getter.
This class can store the property names using the StringConcatFactory format and all the getters (the constant method handles).

A kick and simple implementation can use Class.getRecordComponents() on the lookup.lookupClass() to populate the instance of RecordMirror.
I will be less efficient than calling one native method that fills the RecordMirror instance but this can be optimized later.

So the bootstrap method is just:
  public static Object bootstrap(Lookup lookup, String methodName, TypeDescriptor type, Object recordMirrorObject) 

The bootstrap can easy validate the recordMirrorObject just by doing a cast to RecordMirror.

regards,
Rémi

----- Mail original -----
> De: "Vicente Romero" <vicente.romero at oracle.com>
> À: "amber-dev" <amber-dev at openjdk.java.net>, "compiler-dev" <compiler-dev at openjdk.java.net>
> Envoyé: Lundi 21 Octobre 2019 23:10:09
> Objet: RFR: JEP 359-Records: reflection code

> Hi,
> 
> Please review the reflection code for JEP 359 (Records) at [1]. There is
> a class here that is special [2] this contain the bootstrap methods for
> equals, hashCode and toString methods in records.
> 
> Thanks in advance for the feedback,
> Vicente
> 
> [1] http://cr.openjdk.java.net/~vromero/records.review/reflection/webrev.00/
> [2]
> http://cr.openjdk.java.net/~vromero/records.review/reflection/webrev.00/src/java.base/share/classes/java/lang/runtime/ObjectMethods.java.html