Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

8338544: Dedicated Array class descriptor implementation #20665

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
+554 −365
Open

8338544: Dedicated Array class descriptor implementation #20665

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0b51431
8338544: Dedicated Array class descriptor implementation
liach Aug 19, 2024
51e9945
Merge branch 'master' of https://github.com/openjdk/jdk into feature/…
liach Aug 21, 2024
d1f1729
Redundant import
liach Aug 21, 2024
a424ada
Compile error
liach Aug 21, 2024
9eb2868
Merge branch 'master' of https://github.com/openjdk/jdk into feature/…
liach Aug 27, 2024
c648ce5
Merge branch 'master' of https://github.com/openjdk/jdk into feature/…
liach Sep 24, 2024
81c2fa0
Cleanup after merge
liach Sep 24, 2024
f13588e
Merge branch 'master' of https://github.com/openjdk/jdk into feature/…
liach Oct 7, 2024
a1d1418
class or interface descriptor renamed for clarity
liach Oct 8, 2024
f6964d1
mlchung suggestions: elementType, improve utility methods
liach Oct 8, 2024
ba51c8d
Merge branch 'master' of https://github.com/openjdk/jdk into feature/…
liach Oct 11, 2024
e65fa6a
Mandy reviews
liach Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 13 additions & 90 deletions src/java.base/share/classes/java/lang/constant/ClassDesc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,13 @@

import java.lang.invoke.MethodHandles;
import java.lang.invoke.TypeDescriptor;
import java.util.stream.Stream;

import jdk.internal.constant.ArrayClassDescImpl;
import jdk.internal.constant.PrimitiveClassDescImpl;
import jdk.internal.constant.ReferenceClassDescImpl;
import sun.invoke.util.Wrapper;

import static java.util.stream.Collectors.joining;
import static jdk.internal.constant.ConstantUtils.MAX_ARRAY_TYPE_DESC_DIMENSIONS;
import static jdk.internal.constant.ConstantUtils.arrayDepth;
import static jdk.internal.constant.ConstantUtils.binaryToInternal;
import static jdk.internal.constant.ConstantUtils.forPrimitiveType;
import static jdk.internal.constant.ConstantUtils.internalToBinary;
import static jdk.internal.constant.ConstantUtils.validateBinaryClassName;
import static jdk.internal.constant.ConstantUtils.validateInternalClassName;
import static jdk.internal.constant.ConstantUtils.validateMemberName;
Expand All @@ -63,7 +58,8 @@ public sealed interface ClassDesc
extends ConstantDesc,
TypeDescriptor.OfField<ClassDesc>
permits PrimitiveClassDescImpl,
ReferenceClassDescImpl {
ReferenceClassDescImpl,
ArrayClassDescImpl {

/**
* Returns a {@linkplain ClassDesc} for a class or interface type,
Expand Down Expand Up @@ -179,20 +175,7 @@ static ClassDesc ofDescriptor(String descriptor) {
* ClassDesc} would have an array rank of greater than 255
* @jvms 4.4.1 The CONSTANT_Class_info Structure
*/
default ClassDesc arrayType() {
String desc = descriptorString();
int depth = arrayDepth(desc);
if (depth >= MAX_ARRAY_TYPE_DESC_DIMENSIONS) {
throw new IllegalStateException(
"Cannot create an array type descriptor with more than " +
MAX_ARRAY_TYPE_DESC_DIMENSIONS + " dimensions");
}
String newDesc = "[".concat(desc);
if (desc.length() == 1 && desc.charAt(0) == 'V') {
throw new IllegalArgumentException("not a valid reference type descriptor: " + newDesc);
}
return ReferenceClassDescImpl.ofValidated(newDesc);
}
ClassDesc arrayType();

/**
* Returns a {@linkplain ClassDesc} for an array type of the specified rank,
Expand All @@ -205,24 +188,7 @@ default ClassDesc arrayType() {
* greater than 255
* @jvms 4.4.1 The CONSTANT_Class_info Structure
*/
default ClassDesc arrayType(int rank) {
if (rank <= 0) {
throw new IllegalArgumentException("rank " + rank + " is not a positive value");
}
String desc = descriptorString();
long currentDepth = arrayDepth(desc);
long netRank = currentDepth + rank;
if (netRank > MAX_ARRAY_TYPE_DESC_DIMENSIONS) {
throw new IllegalArgumentException("rank: " + netRank +
" exceeds maximum supported dimension of " +
MAX_ARRAY_TYPE_DESC_DIMENSIONS);
}
String newDesc = new StringBuilder(desc.length() + rank).repeat('[', rank).append(desc).toString();
if (desc.length() == 1 && desc.charAt(0) == 'V') {
throw new IllegalArgumentException("not a valid reference type descriptor: " + newDesc);
}
return ReferenceClassDescImpl.ofValidated(newDesc);
}
ClassDesc arrayType(int rank);

/**
* Returns a {@linkplain ClassDesc} for a nested class of the class or
Expand All @@ -242,13 +208,7 @@ default ClassDesc arrayType(int rank) {
* @throws IllegalArgumentException if the nested class name is invalid
*/
default ClassDesc nested(String nestedName) {
validateMemberName(nestedName, false);
if (!isClassOrInterface())
throw new IllegalStateException("Outer class is not a class or interface type");
String desc = descriptorString();
StringBuilder sb = new StringBuilder(desc.length() + nestedName.length() + 1);
sb.append(desc, 0, desc.length() - 1).append('$').append(nestedName).append(';');
return ReferenceClassDescImpl.ofValidated(sb.toString());
throw new IllegalStateException("Outer class is not a class or interface type");
}

/**
Expand All @@ -265,16 +225,7 @@ default ClassDesc nested(String nestedName) {
* @throws IllegalArgumentException if the nested class name is invalid
*/
default ClassDesc nested(String firstNestedName, String... moreNestedNames) {
if (!isClassOrInterface())
throw new IllegalStateException("Outer class is not a class or interface type");
validateMemberName(firstNestedName, false);
// implicit null-check
for (String addNestedNames : moreNestedNames) {
validateMemberName(addNestedNames, false);
}
return moreNestedNames.length == 0
? nested(firstNestedName)
: nested(firstNestedName + Stream.of(moreNestedNames).collect(joining("$", "$", "")));
throw new IllegalStateException("Outer class is not a class or interface type");
}

/**
Expand All @@ -283,7 +234,7 @@ default ClassDesc nested(String firstNestedName, String... moreNestedNames) {
* @return whether this {@linkplain ClassDesc} describes an array type
*/
default boolean isArray() {
return descriptorString().charAt(0) == '[';
return false;
liach marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -292,7 +243,7 @@ default boolean isArray() {
* @return whether this {@linkplain ClassDesc} describes a primitive type
*/
default boolean isPrimitive() {
return descriptorString().length() == 1;
return false;
liach marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -301,7 +252,7 @@ default boolean isPrimitive() {
* @return whether this {@linkplain ClassDesc} describes a class or interface type
*/
default boolean isClassOrInterface() {
return descriptorString().charAt(0) == 'L';
return false;
liach marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -312,14 +263,6 @@ default boolean isClassOrInterface() {
* if this descriptor does not describe an array type
*/
default ClassDesc componentType() {
if (isArray()) {
String desc = descriptorString();
if (desc.length() == 2) {
return Wrapper.forBasicType(desc.charAt(1)).basicClassDescriptor();
} else {
return ReferenceClassDescImpl.ofValidated(desc.substring(1));
}
}
return null;
}

Expand All @@ -331,41 +274,21 @@ default ClassDesc componentType() {
* default package, or this {@linkplain ClassDesc} does not describe a class or interface type
*/
default String packageName() {
if (!isClassOrInterface())
return "";
String desc = descriptorString();
int index = desc.lastIndexOf('/');
return (index == -1) ? "" : internalToBinary(desc.substring(1, index));
return "";
}

/**
* Returns a human-readable name for the type described by this descriptor.
*
* @implSpec
* <p>The default implementation returns the simple name
* <p>The implementations return the simple name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* <p>The implementations return the simple name
* <p>The implementation returns the simple name

I think this can be singular as it's not required to have more than 1 implementation class.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, in fact, should we still have this @implSpec if we make this default?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be dropped as ClassDesc as well. It can say "This method returns....". See what CSR review would say.

* (e.g., {@code int}) for primitive types, the unqualified class name
* for class or interface types, or the display name of the component type
* suffixed with the appropriate number of {@code []} pairs for array types.
*
* @return the human-readable name
*/
default String displayName() {
if (isPrimitive())
return Wrapper.forBasicType(descriptorString().charAt(0)).primitiveSimpleName();
else if (isClassOrInterface()) {
String desc = descriptorString();
return desc.substring(Math.max(1, desc.lastIndexOf('/') + 1), desc.length() - 1);
}
else if (isArray()) {
int depth = arrayDepth(descriptorString());
ClassDesc c = this;
for (int i=0; i<depth; i++)
c = c.componentType();
return c.displayName() + "[]".repeat(depth);
}
else
throw new IllegalStateException(descriptorString());
}
String displayName();

/**
* Returns a field type descriptor string for this type
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/invoke/InnerClassLambdaMetafactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ private static class SerializationSupport {
static final ClassDesc CD_NotSerializableException = ReferenceClassDescImpl.ofValidated("Ljava/io/NotSerializableException;");
static final MethodTypeDesc MTD_CTOR_NOT_SERIALIZABLE_EXCEPTION = MethodTypeDescImpl.ofValidated(CD_void, CD_String);
static final MethodTypeDesc MTD_CTOR_SERIALIZED_LAMBDA = MethodTypeDescImpl.ofValidated(CD_void,
CD_Class, CD_String, CD_String, CD_String, CD_int, CD_String, CD_String, CD_String, CD_String, ReferenceClassDescImpl.ofValidated("[Ljava/lang/Object;"));
CD_Class, CD_String, CD_String, CD_String, CD_int, CD_String, CD_String, CD_String, CD_String, CD_Object.arrayType());

}

Expand Down
6 changes: 3 additions & 3 deletions src/java.base/share/classes/java/lang/invoke/InvokerBytecodeGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ class InvokerBytecodeGenerator {
private static final ClassDesc CD_LambdaForm = ReferenceClassDescImpl.ofValidated("Ljava/lang/invoke/LambdaForm;");
private static final ClassDesc CD_LambdaForm_Name = ReferenceClassDescImpl.ofValidated("Ljava/lang/invoke/LambdaForm$Name;");
private static final ClassDesc CD_LoopClauses = ReferenceClassDescImpl.ofValidated("Ljava/lang/invoke/MethodHandleImpl$LoopClauses;");
private static final ClassDesc CD_Object_array = ReferenceClassDescImpl.ofValidated("[Ljava/lang/Object;");
private static final ClassDesc CD_MethodHandle_array = ReferenceClassDescImpl.ofValidated("[Ljava/lang/invoke/MethodHandle;");
private static final ClassDesc CD_MethodHandle_array2 = ReferenceClassDescImpl.ofValidated("[[Ljava/lang/invoke/MethodHandle;");
private static final ClassDesc CD_Object_array = CD_Object.arrayType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess CD_Object.arrayType() shows up often enough now - even once in java.lang.constant.ConstantDescs - that we might as well pin it down as a constant somewhere (ConstantDescs is a candidate location, but that will take a CSR).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch already has a CSR for trivial signature changes. The real difficulty lies in how we should name our new array class descriptors, Object_array or ObjectArray or what else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, can you leave a quick review on CSR https://bugs.openjdk.org/browse/JDK-8340963 too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I will do this in another patch that adds it to ConstantDescs - there's a place in ConstantDescs that could have used it, but if I put it in ConstantUtils I fear we are at risk of circular initialization dependencies.

private static final ClassDesc CD_MethodHandle_array = CD_MethodHandle.arrayType();
private static final ClassDesc CD_MethodHandle_array2 = CD_MethodHandle_array.arrayType();

private static final MethodTypeDesc MTD_boolean_Object = MethodTypeDescImpl.ofValidated(CD_boolean, CD_Object);
private static final MethodTypeDesc MTD_Object_int = MethodTypeDescImpl.ofValidated(CD_Object, CD_int);
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/invoke/MethodHandleImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ static MethodHandle bindCaller(MethodHandle mh, Class<?> hostClass) {
// That way we can lazily load the code and set up the constants.
private static class BindCaller {

private static final ClassDesc CD_Object_array = ReferenceClassDescImpl.ofValidated("[Ljava/lang/Object;");
private static final ClassDesc CD_Object_array = CD_Object.arrayType();
private static final MethodType INVOKER_MT = MethodType.methodType(Object.class, MethodHandle.class, Object[].class);
private static final MethodType REFLECT_INVOKER_MT = MethodType.methodType(Object.class, MethodHandle.class, Object.class, Object[].class);

Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/lang/reflect/ProxyGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ final class ProxyGenerator {

private static final ClassDesc
CD_ClassLoader = ReferenceClassDescImpl.ofValidated("Ljava/lang/ClassLoader;"),
CD_Class_array = ReferenceClassDescImpl.ofValidated("[Ljava/lang/Class;"),
CD_Class_array = CD_Class.arrayType(),
CD_ClassNotFoundException = ReferenceClassDescImpl.ofValidated("Ljava/lang/ClassNotFoundException;"),
CD_NoClassDefFoundError = ReferenceClassDescImpl.ofValidated("Ljava/lang/NoClassDefFoundError;"),
CD_IllegalAccessException = ReferenceClassDescImpl.ofValidated("Ljava/lang/IllegalAccessException;"),
CD_InvocationHandler = ReferenceClassDescImpl.ofValidated("Ljava/lang/reflect/InvocationHandler;"),
CD_Method = ReferenceClassDescImpl.ofValidated("Ljava/lang/reflect/Method;"),
CD_NoSuchMethodError = ReferenceClassDescImpl.ofValidated("Ljava/lang/NoSuchMethodError;"),
CD_NoSuchMethodException = ReferenceClassDescImpl.ofValidated("Ljava/lang/NoSuchMethodException;"),
CD_Object_array = ReferenceClassDescImpl.ofValidated("[Ljava/lang/Object;"),
CD_Object_array = CD_Object.arrayType(),
CD_Proxy = ReferenceClassDescImpl.ofValidated("Ljava/lang/reflect/Proxy;"),
CD_UndeclaredThrowableException = ReferenceClassDescImpl.ofValidated("Ljava/lang/reflect/UndeclaredThrowableException;");

Expand Down
16 changes: 8 additions & 8 deletions src/java.base/share/classes/jdk/internal/classfile/impl/StackMapGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1248,14 +1248,14 @@ private static record Type(int tag, ClassDesc sym, int bci) {
//frequently used types to reduce footprint
static final Type OBJECT_TYPE = referenceType(CD_Object),
THROWABLE_TYPE = referenceType(CD_Throwable),
INT_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[I")),
BOOLEAN_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[Z")),
BYTE_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[B")),
CHAR_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[C")),
SHORT_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[S")),
LONG_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[J")),
DOUBLE_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[D")),
FLOAT_ARRAY_TYPE = referenceType(ReferenceClassDescImpl.ofValidated("[F")),
INT_ARRAY_TYPE = referenceType(CD_int.arrayType()),
BOOLEAN_ARRAY_TYPE = referenceType(CD_boolean.arrayType()),
BYTE_ARRAY_TYPE = referenceType(CD_byte.arrayType()),
CHAR_ARRAY_TYPE = referenceType(CD_char.arrayType()),
SHORT_ARRAY_TYPE = referenceType(CD_short.arrayType()),
LONG_ARRAY_TYPE = referenceType(CD_long.arrayType()),
DOUBLE_ARRAY_TYPE = referenceType(CD_double.arrayType()),
FLOAT_ARRAY_TYPE = referenceType(CD_float.arrayType()),
STRING_TYPE = referenceType(CD_String),
CLASS_TYPE = referenceType(CD_Class),
METHOD_HANDLE_TYPE = referenceType(CD_MethodHandle),
Expand Down
Loading