infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-03-14 18:03:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

8309413: Improve the performance of MethodTypeDesc::descriptorString

Browse Source 
8304932: MethodTypeDescImpl can be mutated by argument passed to MethodTypeDesc.of

Reviewed-by: mchung
This commit is contained in:
Chen Liang 2023-06-06 15:56:36 +00:00 committed by Mandy Chung
parent 7edd0540e0
commit 38cef2adbd
4 changed files with 239 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/java.base/share/classes/java/lang/constant/MethodTypeDesc.java
View File

@ -64,7 +64,7 @@ public sealed interface MethodTypeDesc
* @since 21
*/
static MethodTypeDesc of(ClassDesc returnDesc) {
return new MethodTypeDescImpl(returnDesc, ConstantUtils.EMPTY_CLASSDESC);
return MethodTypeDescImpl.ofTrusted(returnDesc, ConstantUtils.EMPTY_CLASSDESC);
}
/**
@ -95,7 +95,7 @@ public sealed interface MethodTypeDesc
* {@link ClassDesc} for {@code void}
*/
static MethodTypeDesc of(ClassDesc returnDesc, ClassDesc... paramDescs) {
return new MethodTypeDescImpl(returnDesc, paramDescs);
return MethodTypeDescImpl.ofTrusted(returnDesc, paramDescs.clone());
}
/**
@ -195,13 +195,7 @@ public sealed interface MethodTypeDesc
* @return the method type descriptor string
* @jvms 4.3.3 Method Descriptors
*/
default String descriptorString() {
return String.format("(%s)%s",
Stream.of(parameterArray())
.map(ClassDesc::descriptorString)
.collect(Collectors.joining()),
returnType().descriptorString());
}
String descriptorString();
/**
* Returns a human-readable descriptor for this method type, using the

71
src/java.base/share/classes/java/lang/constant/MethodTypeDescImpl.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -24,6 +24,8 @@
*/
package java.lang.constant;
import jdk.internal.vm.annotation.Stable;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.security.AccessController;
@ -31,6 +33,7 @@ import java.security.PrivilegedAction;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.StringJoiner;
import static java.util.Objects.requireNonNull;
@ -41,22 +44,38 @@ import static java.util.Objects.requireNonNull;
*/
final class MethodTypeDescImpl implements MethodTypeDesc {
private final ClassDesc returnType;
private final ClassDesc[] argTypes;
private final @Stable ClassDesc[] argTypes;
private @Stable String cachedDescriptorString;
/**
* Constructs a {@linkplain MethodTypeDesc} with the specified return type
* and parameter types
* and a trusted and already-validated parameter types array.
*
* @param returnType a {@link ClassDesc} describing the return type
* @param argTypes {@link ClassDesc}s describing the parameter types
* @param validatedArgTypes {@link ClassDesc}s describing the trusted and validated parameter types
*/
MethodTypeDescImpl(ClassDesc returnType, ClassDesc[] argTypes) {
private MethodTypeDescImpl(ClassDesc returnType, ClassDesc[] validatedArgTypes) {
this.returnType = requireNonNull(returnType);
this.argTypes = requireNonNull(argTypes);
this.argTypes = requireNonNull(validatedArgTypes);
}
for (ClassDesc cr : argTypes)
if (cr.isPrimitive() && cr.descriptorString().equals("V"))
/**
* Constructs a {@linkplain MethodTypeDesc} with the specified return type
* and a trusted parameter types array, which will be validated.
*
* @param returnType a {@link ClassDesc} describing the return type
* @param trustedArgTypes {@link ClassDesc}s describing the trusted parameter types
*/
static MethodTypeDescImpl ofTrusted(ClassDesc returnType, ClassDesc[] trustedArgTypes) {
Objects.requireNonNull(returnType);
if (trustedArgTypes.length == 0) // implicit null check
return new MethodTypeDescImpl(returnType, ConstantUtils.EMPTY_CLASSDESC);
for (ClassDesc cd : trustedArgTypes)
if (cd.isPrimitive() && cd.descriptorString().charAt(0) == 'V') // implicit null check
throw new IllegalArgumentException("Void parameters not permitted");
return new MethodTypeDescImpl(returnType, trustedArgTypes);
}
/**
@ -70,9 +89,18 @@ final class MethodTypeDescImpl implements MethodTypeDesc {
*/
static MethodTypeDescImpl ofDescriptor(String descriptor) {
requireNonNull(descriptor);
List<String> types = ConstantUtils.parseMethodDescriptor(descriptor);
ClassDesc[] paramTypes = types.stream().skip(1).map(ClassDesc::ofDescriptor).toArray(ClassDesc[]::new);
return new MethodTypeDescImpl(ClassDesc.ofDescriptor(types.get(0)), paramTypes);
int paramCount = types.size() - 1;
var paramTypes = paramCount > 0 ? new ClassDesc[paramCount] : ConstantUtils.EMPTY_CLASSDESC;
for (int i = 0; i < paramCount; i++) {
paramTypes[i] = ClassDesc.ofDescriptor(types.get(i + 1));
}
MethodTypeDescImpl result = ofTrusted(ClassDesc.ofDescriptor(types.getFirst()), paramTypes);
result.cachedDescriptorString = descriptor;
return result;
}
@Override
@ -102,14 +130,14 @@ final class MethodTypeDescImpl implements MethodTypeDesc {
@Override
public MethodTypeDesc changeReturnType(ClassDesc returnType) {
return MethodTypeDesc.of(returnType, argTypes);
return new MethodTypeDescImpl(returnType, argTypes);
}
@Override
public MethodTypeDesc changeParameterType(int index, ClassDesc paramType) {
ClassDesc[] newArgs = argTypes.clone();
newArgs[index] = paramType;
return MethodTypeDesc.of(returnType, newArgs);
return ofTrusted(returnType, newArgs);
}
@Override
@ -120,18 +148,33 @@ final class MethodTypeDescImpl implements MethodTypeDesc {
ClassDesc[] newArgs = new ClassDesc[argTypes.length - (end - start)];
System.arraycopy(argTypes, 0, newArgs, 0, start);
System.arraycopy(argTypes, end, newArgs, start, argTypes.length - end);
return MethodTypeDesc.of(returnType, newArgs);
return ofTrusted(returnType, newArgs);
}
@Override
public MethodTypeDesc insertParameterTypes(int pos, ClassDesc... paramTypes) {
if (pos < 0 || pos > argTypes.length)
throw new IndexOutOfBoundsException(pos);
ClassDesc[] newArgs = new ClassDesc[argTypes.length + paramTypes.length];
System.arraycopy(argTypes, 0, newArgs, 0, pos);
System.arraycopy(paramTypes, 0, newArgs, pos, paramTypes.length);
System.arraycopy(argTypes, pos, newArgs, pos+paramTypes.length, argTypes.length - pos);
return MethodTypeDesc.of(returnType, newArgs);
return ofTrusted(returnType, newArgs);
}
@Override
public String descriptorString() {
var desc = this.cachedDescriptorString;
if (desc != null)
return desc;
var sj = new StringJoiner("", "(", ")" + returnType().descriptorString());
for (int i = 0; i < parameterCount(); i++) {
sj.add(parameterType(i).descriptorString());
}
return cachedDescriptorString = sj.toString();
}
@Override

137
test/jdk/java/lang/constant/MethodTypeDescTest.java
View File

@ -24,6 +24,7 @@
import java.lang.invoke.MethodType;
import java.lang.constant.ClassDesc;
import java.lang.constant.MethodTypeDesc;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@ -32,16 +33,14 @@ import java.util.stream.Stream;
import org.testng.annotations.Test;
import static java.lang.constant.ConstantDescs.CD_int;
import static java.lang.constant.ConstantDescs.CD_void;
import static java.lang.constant.ConstantDescs.*;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertThrows;
import static org.testng.Assert.fail;
import static org.testng.Assert.*;
/**
* @test
* @bug 8304932
* @compile MethodTypeDescTest.java
* @run testng MethodTypeDescTest
* @summary unit tests for java.lang.constant.MethodTypeDesc
@ -115,12 +114,9 @@ public class MethodTypeDescTest extends SymbolicDescTest {
}
// try with null parameter
try {
expectThrows(NullPointerException.class, () -> {
MethodTypeDesc newDesc = mtDesc.changeReturnType(null);
fail("should fail with NPE");
} catch (NullPointerException ex) {
// good
}
});
// changeParamType
for (int i=0; i<paramTypes.length; i++) {
@ -144,6 +140,15 @@ public class MethodTypeDescTest extends SymbolicDescTest {
MethodTypeDesc newDesc = mtDesc.dropParameterTypes(i, i + 1);
assertEquals(newDesc, MethodTypeDesc.of(returnType, ps));
testMethodTypeDesc(newDesc, mt.dropParameterTypes(i, i+1));
// drop multiple params
for (int j = i; j < paramTypes.length; j++) {
var t = new ArrayList<>(Arrays.asList(paramTypes));
t.subList(i, j).clear();
MethodTypeDesc multiDrop = mtDesc.dropParameterTypes(i, j);
assertEquals(multiDrop, MethodTypeDesc.of(returnType, t.toArray(ClassDesc[]::new)));
testMethodTypeDesc(multiDrop, mt.dropParameterTypes(i, j));
}
}
badDropParametersTypes(CD_void, paramDescs);
@ -159,6 +164,21 @@ public class MethodTypeDescTest extends SymbolicDescTest {
assertEquals(newDesc, MethodTypeDesc.of(returnType, ps));
testMethodTypeDesc(newDesc, mt.insertParameterTypes(i, p.resolveConstantDesc(LOOKUP)));
}
// add multiple params
ClassDesc[] addition = {CD_int, CD_String};
var a = new ArrayList<>(Arrays.asList(paramTypes));
a.addAll(i, Arrays.asList(addition));
MethodTypeDesc newDesc = mtDesc.insertParameterTypes(i, addition);
assertEquals(newDesc, MethodTypeDesc.of(returnType, a.toArray(ClassDesc[]::new)));
testMethodTypeDesc(newDesc, mt.insertParameterTypes(i, Arrays.stream(addition).map(d -> {
try {
return (Class<?>) d.resolveConstantDesc(LOOKUP);
} catch (ReflectiveOperationException ex) {
throw new RuntimeException(ex);
}
}).toArray(Class[]::new)));
}
badInsertParametersTypes(CD_void, paramDescs);
@ -169,47 +189,32 @@ public class MethodTypeDescTest extends SymbolicDescTest {
IntStream.rangeClosed(0, paramDescTypes.length - 1)
.mapToObj(i -> ClassDesc.ofDescriptor(paramDescTypes[i])).toArray(ClassDesc[]::new);
MethodTypeDesc mtDesc = MethodTypeDesc.of(returnType, paramTypes);
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.insertParameterTypes(-1, paramTypes);
fail("pos < 0 should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.insertParameterTypes(paramTypes.length + 1, paramTypes);
fail("pos > current arguments length should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
try {
expectThrows(IllegalArgumentException.class, () -> {
ClassDesc[] newParamTypes = new ClassDesc[1];
newParamTypes[0] = CD_void;
MethodTypeDesc newDesc = MethodTypeDesc.of(returnType, CD_int);
newDesc = newDesc.insertParameterTypes(0, newParamTypes);
fail("shouldn't allow parameters with class descriptor CD_void");
} catch (IllegalArgumentException ex) {
// good
}
});
try {
expectThrows(NullPointerException.class, () -> {
MethodTypeDesc newDesc = MethodTypeDesc.of(returnType, CD_int);
newDesc = newDesc.insertParameterTypes(0, null);
fail("should fail with NPE");
} catch (NullPointerException ex) {
// good
}
});
try {
expectThrows(NullPointerException.class, () -> {
ClassDesc[] newParamTypes = new ClassDesc[1];
newParamTypes[0] = null;
MethodTypeDesc newDesc = MethodTypeDesc.of(returnType, CD_int);
newDesc = newDesc.insertParameterTypes(0, newParamTypes);
fail("should fail with NPE");
} catch (NullPointerException ex) {
// good
}
});
}
private void badDropParametersTypes(ClassDesc returnType, String... paramDescTypes) {
@ -217,40 +222,26 @@ public class MethodTypeDescTest extends SymbolicDescTest {
IntStream.rangeClosed(0, paramDescTypes.length - 1)
.mapToObj(i -> ClassDesc.ofDescriptor(paramDescTypes[i])).toArray(ClassDesc[]::new);
MethodTypeDesc mtDesc = MethodTypeDesc.of(returnType, paramTypes);
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.dropParameterTypes(-1, 0);
fail("start index < 0 should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.dropParameterTypes(paramTypes.length, 0);
fail("start index = arguments.length should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.dropParameterTypes(paramTypes.length + 1, 0);
fail("start index > arguments.length should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.dropParameterTypes(0, paramTypes.length + 1);
fail("end index > arguments.length should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
try {
expectThrows(IndexOutOfBoundsException.class, () -> {
MethodTypeDesc newDesc = mtDesc.dropParameterTypes(1, 0);
fail("start index > end index should have failed");
} catch (IndexOutOfBoundsException ex) {
// good
}
});
}
public void testMethodTypeDesc() throws ReflectiveOperationException {
@ -274,6 +265,7 @@ public class MethodTypeDescTest extends SymbolicDescTest {
for (String d : badDescriptors) {
assertThrows(IllegalArgumentException.class, () -> MethodTypeDesc.ofDescriptor(d));
}
assertThrows(NullPointerException.class, () -> MethodTypeDesc.ofDescriptor(null));
// of(ClassDesc)
@ -282,8 +274,6 @@ public class MethodTypeDescTest extends SymbolicDescTest {
// of(ClassDesc, ClassDesc...)
assertThrows(NullPointerException.class, () -> MethodTypeDesc.of(CD_int, (ClassDesc[]) null));
assertThrows(NullPointerException.class, () -> MethodTypeDesc.of(CD_int, new ClassDesc[] {null}));
// try with void arguments, this will stress another code path in particular
// ConstantMethodTypeDesc::init
assertThrows(IllegalArgumentException.class, () -> MethodTypeDesc.of(CD_int, CD_void));
// of(ClassDesc, List<ClassDesc>)
@ -291,4 +281,27 @@ public class MethodTypeDescTest extends SymbolicDescTest {
assertThrows(NullPointerException.class, () -> MethodTypeDesc.of(CD_int, Collections.singletonList(null)));
assertThrows(IllegalArgumentException.class, () -> MethodTypeDesc.of(CD_int, List.of(CD_void)));
}
public void testOfArrayImmutability() {
ClassDesc[] args = {CD_Object, CD_int};
var mtd = MethodTypeDesc.of(CD_void, args);
args[1] = CD_void;
assertEquals(mtd, MethodTypeDesc.of(CD_void, CD_Object, CD_int));
mtd.parameterArray()[1] = CD_void;
assertEquals(mtd, MethodTypeDesc.of(CD_void, CD_Object, CD_int));
}
public void testOfListImmutability() {
List<ClassDesc> args = Arrays.asList(CD_Object, CD_int);
var mtd = MethodTypeDesc.of(CD_void, args);
args.set(1, CD_void);
assertEquals(mtd, MethodTypeDesc.of(CD_void, CD_Object, CD_int));
assertThrows(UnsupportedOperationException.class, () ->
mtd.parameterList().set(1, CD_void));
assertEquals(mtd, MethodTypeDesc.of(CD_void, CD_Object, CD_int));
}
}

104
test/micro/org/openjdk/bench/java/lang/constant/MethodTypeDescFactories.java Normal file
View File

@ -0,0 +1,104 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.openjdk.bench.java.lang.constant;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;
import java.lang.constant.ClassDesc;
import java.lang.constant.MethodTypeDesc;
import java.util.List;
import java.util.concurrent.TimeUnit;
/**
* Performance of conversion from and to method type descriptor symbols with
* descriptor strings and class descriptor symbols
*/
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 3, time = 2)
@Measurement(iterations = 6, time = 1)
@Fork(1)
@State(Scope.Benchmark)
public class MethodTypeDescFactories {
private static final ClassDesc DUMMY_CD = ClassDesc.of("Dummy_invalid");
@Param({
"(Ljava/lang/Object;Ljava/lang/String;)I",
"()V",
"([IJLjava/lang/String;Z)Ljava/util/List;",
"()[Ljava/lang/String;"
})
public String descString;
public MethodTypeDesc desc;
public ClassDesc ret;
public ClassDesc[] args;
public List<ClassDesc> argsList;
@Setup
public void setup() {
desc = MethodTypeDesc.ofDescriptor(descString);
ret = desc.returnType();
args = desc.parameterArray();
argsList = desc.parameterList();
}
@Benchmark
public String descriptorString(Blackhole blackhole) {
// swaps return types with dummy classdesc;
// this shares parameter arrays and avoids revalidation
// while it drops the descriptor string cache
var mtd = desc.changeReturnType(DUMMY_CD);
blackhole.consume(mtd);
mtd = mtd.changeReturnType(desc.returnType());
blackhole.consume(mtd);
return mtd.descriptorString();
}
@Benchmark
public MethodTypeDesc ofDescriptor() {
return MethodTypeDesc.ofDescriptor(descString);
}
@Benchmark
public MethodTypeDesc ofArray() {
return MethodTypeDesc.of(ret, args);
}
@Benchmark
public MethodTypeDesc ofList() {
return MethodTypeDesc.of(ret, argsList);
}
}