Changing the bootstrap protocol of ObjectMethods
forax at univ-mlv.fr
forax at univ-mlv.fr
Fri Feb 7 16:57:32 UTC 2020
> De: "Brian Goetz" <brian.goetz at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>, "amber-dev" <amber-dev at openjdk.java.net>
> Envoyé: Vendredi 7 Février 2020 00:42:06
> Objet: Re: Changing the bootstrap protocol of ObjectMethods
> At the time, I recall asking "But, what is the point? What problem are you
> actually trying to solve?" I am still hazy on this. There's nothing _wrong_
> with the design you're suggesting, but I am having a hard time seeing how it is
> better, or that there's something wrong with what we've got that needs
> changing.
>> This changes
>> - separate the API for the record used by javac from the implementation of
>> equals/hashCode and toString allowing other JVM languages to reuse the
>> implementation even for classes that are non record
> I still don't see the problem. For a non-record class, there is still,
> abstractly, a set of "state variables"; they can each be accessed with a method
> handle (field accessor, getter method, etc), and they each have a name that
> will be used in the toString. How is the current bootstrap unsuitable for
> non-record classes? With the current bootstrap, it is equally useful to records
> and non-records alike, with no bias towards records. Surely that is better than
> having two separate bootstraps, one for records and one for everyone else?
>> - for that a new class RecordLikeMirror is introduced as an abstraction on
>> anything that defines components with a name and a type.
> OK, another new abstraction. What value does it serve?
>> - 3 new public API points are created,
>> createEquals/createHashCode/createToString
> This is a valid direction, but again I think it's unnecessary. We have the the
> name/type channel in indy whether we use it or not; having three entry points
> which have to be separately specified and tested doesn't seem simpler than one.
> Further, the current mechanism is extensible to other methods that can be
> generated from the same metadata (such as, perhaps, compareTo), whereas three
> separate bootstraps would not be. Yes, the equals/hashCode bootstrap could take
> one fewer parameter than toString, but even this is not a win -- because then
> the BMT entry can't be shared.
>> - so now the method "bootstrap" can be specialized only for records and use
>> reflection to extract the record components instead passing them as bootstrap
>> arguments
> That seems like a negative, not a positive?
> Can we back up? What problem are we solving?
There are mostly three problems with the actual protocol:
- information duplication, the actual classfile contains twice the same abstract description of what a record is, in the Record attribute and in the list of bootstrap arguments the method ''bootstrap", unifying them is better in the long term because we don't have to deal with de-synchronization of them.
- information hiding, the current protocol shows the array of method handles in plain sight, in the signature of the BSM, so we can not to change the implementation without asking for recompiling all record classfiles, which is bad in the long term.
- the validation of those arguments is done once per indy callsite instead of once per class using a condy.
The class RecordLikeMirror has several purposes :
- it hides from where the info comes from, the Record attribute or some user provided data (from another language runtime)
- its factory methods check that the arguments are valid
- it's used has the return type of condy so the validation is done once per class.
I agree with you that the 3 API entry-points are not necessary because one can call the method "bootstrap" instead.
So i've remove them from the patch (below).
cheers,
Rémi
---
diff -r 1c4286ec9e45 src/java.base/share/classes/java/lang/runtime/ObjectMethods.java
--- a/src/java.base/share/classes/java/lang/runtime/ObjectMethods.java Wed Feb 05 10:14:40 2020 +0100
+++ b/src/java.base/share/classes/java/lang/runtime/ObjectMethods.java Fri Feb 07 16:49:47 2020 +0100
@@ -29,8 +29,9 @@
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.invoke.TypeDescriptor;
+import java.lang.invoke.WrongMethodTypeException;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Arrays;
import java.util.HashMap;
@@ -298,78 +299,223 @@
return formatter;
}
/**
+ * A class that represent a record-like class, with the component names and accessor methods.
+ */
+ public static final class RecordLikeMirror {
+ private final Class<?> type;
+ private final List<String> names;
+ private final List<MethodHandle> accessors;
+
+ private RecordLikeMirror(Class<?> type, List<String> names, List<MethodHandle> accessors) {
+ this.type = type;
+ this.names = names;
+ this.accessors = accessors;
+ }
+
+ /**
+ * Create a record-like mirror from a class a list of names and a list of
+ * accessors.
+ *
+ * @param type the class to mirror.
+ * @param names the names of the components.
+ * @param accessors the accessors of the components.
+ * @return a new record-like mirror.
+ */
+ public static RecordLikeMirror of(Class<?> type, List<String> names, List<MethodHandle> accessors) {
+ Objects.requireNonNull(type);
+ List<String> nameList = List.copyOf(names);
+ List<MethodHandle> accessorList = List.copyOf(accessors);
+ if (nameList.size() != accessorList.size()) {
+ throw new IllegalArgumentException("names.size() != accessors.size()");
+ }
+ for(MethodHandle accessor: accessorList) {
+ MethodType methodType = accessor.type();
+ if (methodType.returnType() == void.class || methodType.parameterCount() != 1 || methodType.parameterType(0) != type) {
+ throw new WrongMethodTypeException(methodType.toString());
+ }
+ }
+ return new RecordLikeMirror(type, nameList, accessorList);
+ }
+
+ /**
+ * Create a record-like mirror from a record.
+ *
+ * @param lookup the lookup used to create the accessor from the record
+ * components.
+ * @param recordClass the record class to mirror.
+ * @return a new record-like mirror.
+ * @throws IllegalAccessException if the lookup can not access to the accessors.
+ */
+ public static RecordLikeMirror fromRecord(MethodHandles.Lookup lookup, Class<?> recordClass) throws IllegalAccessException {
+ Objects.requireNonNull(lookup);
+ @SuppressWarnings("preview")
+ var recordComponents = recordClass.getRecordComponents(); // implicit nullcheck
+ if (recordComponents == null) {
+ throw new IllegalArgumentException("the class taken as argument is not a record");
+ }
+ int length = recordComponents.length;
+ String[] names = new String[length];
+ MethodHandle[] accessors = new MethodHandle[length];
+ for(int i = 0; i < length; i++) {
+ var recordComponent = recordComponents[i];
+ names[i] = recordComponent.getName();
+ accessors[i] = lookup.unreflect(recordComponent.getAccessor());
+ }
+ return new RecordLikeMirror(recordClass, List.of(names), List.of(accessors));
+ }
+ }
+
+ /**
+ * A bootstrap method for a constant dynamic that creates a record-like mirror
+ * from a record class.
+ *
+ * @param lookup the lookup used to retrieve the accessors.
+ * @param name any name.
+ * @param type the type of the constant ({@code RecordLikeMirror}).
+ * @param recordClass the record class.
+ * @return a record-like mirror created from the recordClass.
+ * @throws IllegalArgumentException if the type is not {@code RecordLikeMirror}.
+ * @throws IllegalAccessError if the lookup can not access to the
+ * accessors.
+ */
+ public static RecordLikeMirror recordLikeMirrorConstant(MethodHandles.Lookup lookup,
+ String name,
+ Class<?> type,
+ Class<?> recordClass) {
+ Objects.requireNonNull(lookup);
+ Objects.requireNonNull(name);
+ Objects.requireNonNull(type);
+ Objects.requireNonNull(recordClass);
+ if (type != RecordLikeMirror.class) {
+ throw new IllegalArgumentException("type should be equals to RecordLikeMirror");
+ }
+ try {
+ return RecordLikeMirror.fromRecord(lookup, recordClass);
+ } catch (IllegalAccessException e) {
+ throw (IllegalAccessError)new IllegalAccessError().initCause(e);
+ }
+ }
+
+ /**
* Bootstrap method to generate the {@link Object#equals(Object)},
- * {@link Object#hashCode()}, and {@link Object#toString()} methods, based
- * on a description of the component names and accessor methods, for either
+ * {@link Object#hashCode()}, and {@link Object#toString()} methods, from a
+ * record-like mirror, for either {@code invokedynamic} call sites or dynamic
+ * constant pool entries.
+ *
+ * For more detail on the semantics of the generated methods see the
+ * specification of {@link java.lang.Record#equals(Object)},
+ * {@link java.lang.Record#hashCode()} and {@link java.lang.Record#toString()}.
+ *
+ * @param lookup Every bootstrap method is expected to have a {@code lookup}
+ * which usually represents a lookup context with the
+ * accessibility privileges of the caller. This is because
+ * {@code invokedynamic} call sites always provide a
+ * {@code lookup} to the corresponding bootstrap method, but
+ * this method just ignores the {@code lookup} parameter
+ * @param methodName the name of the method to generate, which must be one of
+ * {@code "equals"}, {@code "hashCode"}, or {@code "toString"}
+ * @param type a {@link MethodType} corresponding the descriptor type for
+ * the method, which must correspond to the descriptor for the
+ * corresponding {@link Object} method, if linking an
+ * {@code invokedynamic} call site, or the constant
+ * {@code MethodHandle.class}, if linking a dynamic constant
+ * @param mirror a description of a record-like class
+ * @return a call site if invoked by indy, or a method handle if invoked by a
+ * condy
+ * @throws IllegalArgumentException if the bootstrap arguments are invalid or
+ * inconsistent
+ */
+ public static Object bootstrap(MethodHandles.Lookup lookup,
+ String methodName,
+ TypeDescriptor type,
+ RecordLikeMirror mirror) {
+ Objects.requireNonNull(lookup);
+ Objects.requireNonNull(methodName);
+ Objects.requireNonNull(type);
+ Objects.requireNonNull(mirror);
+ MethodType methodType;
+ if (type instanceof MethodType) {
+ methodType = (MethodType) type;
+ } else {
+ if (MethodHandle.class != type) {
+ throw new IllegalArgumentException(type.toString());
+ }
+ methodType = null;
+ }
+ MethodHandle handle = switch (methodName) {
+ case "equals" -> {
+ if (methodType != null &&
+ (methodType.returnType() != boolean.class || methodType.parameterCount() != 2 ||
+ methodType.parameterType(0) != mirror.type || methodType.parameterType(1) != Object.class)) {
+ throw new IllegalArgumentException("Bad method type: " + methodType);
+ }
+ yield makeEquals(mirror.type, mirror.accessors);
+ }
+ case "hashCode" -> {
+ if (methodType != null &&
+ (methodType.returnType() != int.class || methodType.parameterCount() != 1 ||
+ methodType.parameterType(0) != mirror.type)) {
+ throw new IllegalArgumentException("Bad method type: " + methodType);
+ }
+ yield makeHashCode(mirror.type, mirror.accessors);
+ }
+ case "toString" -> {
+ if (methodType != null &&
+ (methodType.returnType() != String.class || methodType.parameterCount() != 1 ||
+ methodType.parameterType(0) != mirror.type)) {
+ throw new IllegalArgumentException("Bad method type: " + methodType);
+ }
+ yield makeToString(mirror.type, mirror.accessors, mirror.names);
+ }
+ default -> throw new IllegalArgumentException(methodName);
+ };
+ return methodType != null ? new ConstantCallSite(handle) : handle;
+ }
+
+ /**
+ * Bootstrap method to generate the {@link Object#equals(Object)},
+ * {@link Object#hashCode()}, and {@link Object#toString()} methods, based on a
+ * description of the component names and accessor methods, for either
* {@code invokedynamic} call sites or dynamic constant pool entries.
*
- * For more detail on the semantics of the generated methods see the specification
- * of {@link java.lang.Record#equals(Object)}, {@link java.lang.Record#hashCode()} and
- * {@link java.lang.Record#toString()}.
- *
+ * For more detail on the semantics of the generated methods see the
+ * specification of {@link java.lang.Record#equals(Object)},
+ * {@link java.lang.Record#hashCode()} and {@link java.lang.Record#toString()}.
*
- * @param lookup Every bootstrap method is expected to have a {@code lookup}
- * which usually represents a lookup context with the
- * accessibility privileges of the caller. This is because
- * {@code invokedynamic} call sites always provide a {@code lookup}
- * to the corresponding bootstrap method, but this method just
- * ignores the {@code lookup} parameter
- * @param methodName the name of the method to generate, which must be one of
- * {@code "equals"}, {@code "hashCode"}, or {@code "toString"}
- * @param type a {@link MethodType} corresponding the descriptor type
- * for the method, which must correspond to the descriptor
- * for the corresponding {@link Object} method, if linking
- * an {@code invokedynamic} call site, or the
- * constant {@code MethodHandle.class}, if linking a
- * dynamic constant
- * @param recordClass the record class hosting the record components
- * @param names the list of component names, joined into a string
- * separated by ";", or the empty string if there are no
- * components. Maybe be null, if the {@code methodName}
- * is {@code "equals"} or {@code "hashCode"}.
- * @param getters method handles for the accessor methods for the components
- * @return a call site if invoked by indy, or a method handle
- * if invoked by a condy
- * @throws IllegalArgumentException if the bootstrap arguments are invalid
- * or inconsistent
- * @throws Throwable if any exception is thrown during call site construction
+ * @param lookup Every bootstrap method is expected to have a
+ * {@code lookup} which usually represents a lookup context
+ * with the accessibility privileges of the caller. This is
+ * because {@code invokedynamic} call sites always provide a
+ * {@code lookup} to the corresponding bootstrap method, but
+ * this method just ignores the {@code lookup} parameter
+ * @param methodName the name of the method to generate, which must be one of
+ * {@code "equals"}, {@code "hashCode"}, or
+ * {@code "toString"}
+ * @param type a {@link MethodType} corresponding the descriptor type for
+ * the method, which must correspond to the descriptor for
+ * the corresponding {@link Object} method, if linking an
+ * {@code invokedynamic} call site, or the constant
+ * {@code MethodHandle.class}, if linking a dynamic constant
+ * @param recordClass the record class hosting the record components
+ * @param names unused.
+ * @param getters unused
+ * @return a call site if invoked by indy, or a method handle if invoked by a
+ * condy
+ * @throws IllegalArgumentException if the bootstrap arguments are invalid or
+ * inconsistent
+ * @throws Throwable if any exception is thrown during call site
+ * construction
+ *
+ * @deprecated this method is deprecated and replaced by
+ * {@link #bootstrap(java.lang.invoke.MethodHandles.Lookup, String, TypeDescriptor, RecordLikeMirror)}
*/
+ @Deprecated(forRemoval = true)
public static Object bootstrap(MethodHandles.Lookup lookup, String methodName, TypeDescriptor type,
- Class<?> recordClass,
- String names,
- MethodHandle... getters) throws Throwable {
- MethodType methodType;
- if (type instanceof MethodType)
- methodType = (MethodType) type;
- else {
- methodType = null;
- if (!MethodHandle.class.equals(type))
- throw new IllegalArgumentException(type.toString());
- }
- List<MethodHandle> getterList = List.of(getters);
- MethodHandle handle;
- switch (methodName) {
- case "equals":
- if (methodType != null && !methodType.equals(MethodType.methodType(boolean.class, recordClass, Object.class)))
- throw new IllegalArgumentException("Bad method type: " + methodType);
- handle = makeEquals(recordClass, getterList);
- return methodType != null ? new ConstantCallSite(handle) : handle;
- case "hashCode":
- if (methodType != null && !methodType.equals(MethodType.methodType(int.class, recordClass)))
- throw new IllegalArgumentException("Bad method type: " + methodType);
- handle = makeHashCode(recordClass, getterList);
- return methodType != null ? new ConstantCallSite(handle) : handle;
- case "toString":
- if (methodType != null && !methodType.equals(MethodType.methodType(String.class, recordClass)))
- throw new IllegalArgumentException("Bad method type: " + methodType);
- List<String> nameList = "".equals(names) ? List.of() : List.of(names.split(";"));
- if (nameList.size() != getterList.size())
- throw new IllegalArgumentException("Name list and accessor list do not match");
- handle = makeToString(recordClass, getterList, nameList);
- return methodType != null ? new ConstantCallSite(handle) : handle;
- default:
- throw new IllegalArgumentException(methodName);
- }
+ Class<?> recordClass,
+ String names,
+ MethodHandle... getters) throws Throwable {
+ RecordLikeMirror mirror = recordLikeMirrorConstant(lookup, "recordLikeMirrorConstant", RecordLikeMirror.class, recordClass);
+ return bootstrap(lookup, methodName, type, mirror);
}
}
More information about the amber-dev
mailing list