-
Notifications
You must be signed in to change notification settings - Fork 38.3k
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.
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
Add fast-path for no-args constructor in BeanUtils.instantiateClass #24104
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ | |
import org.springframework.util.Assert; | ||
import org.springframework.util.ClassUtils; | ||
import org.springframework.util.ConcurrentReferenceHashMap; | ||
import org.springframework.util.ObjectUtils; | ||
import org.springframework.util.ReflectionUtils; | ||
import org.springframework.util.StringUtils; | ||
|
||
|
@@ -182,11 +183,18 @@ public static <T> T instantiateClass(Constructor<T> ctor, Object... args) throws | |
try { | ||
ReflectionUtils.makeAccessible(ctor); | ||
if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isKotlinType(ctor.getDeclaringClass())) { | ||
if (ObjectUtils.isEmpty(args)) { | ||
return KotlinDelegate.instantiateClass(ctor); | ||
} | ||
return KotlinDelegate.instantiateClass(ctor, args); | ||
} | ||
else { | ||
int constructorParamCount = ctor.getParameterCount(); | ||
Assert.isTrue(args.length <= constructorParamCount, "Can't specify more arguments than constructor parameters"); | ||
if (constructorParamCount == 0) { | ||
return ctor.newInstance(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please provide a JMH benchmark of the gains? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'll try to. The immediate gain here is that we don't allocate a new array calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am still interested by concrete data points with JMH benchmarks before and after the proposed change, could you please provide that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's the benchmark: @State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
public class MyBenchmark {
private Constructor<TestClass1> noArgConstructor;
private Constructor<TestClass2> constructor;
@Setup
public void setUp() throws NoSuchMethodException {
noArgConstructor = TestClass1.class.getDeclaredConstructor();
constructor = TestClass2.class.getDeclaredConstructor(int.class, String.class);
}
@Benchmark
public Object emptyConstructor() {
return BeanUtils.instantiateClass(noArgConstructor);
}
@Benchmark
public Object nonEmptyConstructor() {
return BeanUtils.instantiateClass(constructor, 1, "str");
}
static class TestClass1{
};
static class TestClass2 {
private final int value1;
private final String value2;
TestClass2(int value1, String value2) {
this.value1 = value1;
this.value2 = value2;
}
}
} And results:
|
||
} | ||
Class<?>[] parameterTypes = ctor.getParameterTypes(); | ||
Assert.isTrue(args.length <= parameterTypes.length, "Can't specify more arguments than constructor parameters"); | ||
Object[] argsWithDefaultValues = new Object[args.length]; | ||
for (int i = 0 ; i < args.length; i++) { | ||
if (args[i] == null) { | ||
|
@@ -788,6 +796,24 @@ public static <T> T instantiateClass(Constructor<T> ctor, Object... args) | |
return kotlinConstructor.callBy(argParameters); | ||
} | ||
|
||
/** | ||
* Instantiate a Kotlin class using provided no-arg constructor. | ||
* @param ctor the constructor of the Kotlin class to instantiate | ||
*/ | ||
public static <T> T instantiateClass(Constructor<T> ctor) | ||
throws IllegalAccessException, InvocationTargetException, InstantiationException { | ||
|
||
KFunction<T> kotlinConstructor = ReflectJvmMapping.getKotlinFunction(ctor); | ||
if (kotlinConstructor == null) { | ||
return ctor.newInstance(); | ||
} | ||
List<KParameter> parameters = kotlinConstructor.getParameters(); | ||
Assert.isTrue(parameters.isEmpty(), "Default no-args constructor must have no params"); | ||
Map<KParameter, Object> argParameters = Collections.emptyMap(); | ||
return kotlinConstructor.callBy(argParameters); | ||
} | ||
|
||
|
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the fact that JetBrains/kotlin#2855 has been closed because in practice it brings no benefit, is that part still relevant from your POV and if yes, could you elaborate on why and provide a JMH benchmark to quantify the gains?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, here's the benchmark:
Results: