From 9eb042bb0cf581224aff56d0a1b1b00b4c75c0b1 Mon Sep 17 00:00:00 2001 From: Elon Azoulay Date: Fri, 1 Mar 2024 05:35:58 -0800 Subject: [PATCH 1/3] Use ConstructorBenchmark in main method --- .../main/java/org/jdbi/v3/benchmark/ConstructorBenchmark.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmark/src/main/java/org/jdbi/v3/benchmark/ConstructorBenchmark.java b/benchmark/src/main/java/org/jdbi/v3/benchmark/ConstructorBenchmark.java index 3551e04c97..f0ec84eaea 100644 --- a/benchmark/src/main/java/org/jdbi/v3/benchmark/ConstructorBenchmark.java +++ b/benchmark/src/main/java/org/jdbi/v3/benchmark/ConstructorBenchmark.java @@ -59,7 +59,7 @@ public class ConstructorBenchmark { public static void main(final String[] args) throws RunnerException { final Options options = new OptionsBuilder() - .include(BeanBindingBenchmark.class.getSimpleName()) + .include(ConstructorBenchmark.class.getSimpleName()) .forks(0) .build(); new Runner(options).run(); From 344bb93c9b44b016d158b40875f327aaa992294f Mon Sep 17 00:00:00 2001 From: Elon Azoulay Date: Thu, 22 Feb 2024 02:03:52 -0800 Subject: [PATCH 2/3] Cache types in ConstructorMapper --- .../reflect/ConstructorInstanceFactory.java | 63 ++++++++++++++++--- .../mapper/reflect/ConstructorMapper.java | 4 +- .../core/mapper/reflect/InstanceFactory.java | 6 +- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java index 06c8c3411f..04b19d1b66 100644 --- a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java +++ b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java @@ -13,38 +13,50 @@ */ package org.jdbi.v3.core.mapper.reflect; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.WeakHashMap; import java.util.function.Supplier; import java.util.stream.Stream; import org.jdbi.v3.core.internal.exceptions.Unchecked; +import static java.util.Collections.synchronizedMap; import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.toUnmodifiableList; class ConstructorInstanceFactory extends InstanceFactory { - private final Constructor constructor; + private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); + private static final Map CONSTRUCTOR_CACHE = synchronizedMap(new WeakHashMap<>()); - private final Supplier typeSupplier; + private final Constructor constructor; + private final List types; + private final MethodHandle constructorHandle; ConstructorInstanceFactory(Constructor constructor) { super(constructor); this.constructor = requireNonNull(constructor, "constructor is null"); - this.typeSupplier = getTypeSupplier(constructor, super::getTypes); + ConstructorHandleAndTypes constructorHandleAndTypes = getConstructorHandleAndTypes(constructor, super::getTypes); + this.types = constructorHandleAndTypes.getTypes(); + this.constructorHandle = constructorHandleAndTypes.getConstructorHandle(); } @Override - Type[] getTypes() { - return typeSupplier.get(); + List getTypes() { + return types; } @Override T newInstance(Object... params) { - return Unchecked.function(constructor::newInstance).apply(params); + return (T) Unchecked.function(constructorHandle::invokeWithArguments).apply(params); } @Override @@ -76,17 +88,48 @@ private static boolean isGenericInformationLost(Constructor factory) { return lossDetected; } - private static Supplier getTypeSupplier(Constructor constructor, Supplier defaultSupplier) { + private static ConstructorHandleAndTypes getConstructorHandleAndTypes(Constructor constructor, Supplier> defaultSupplier) { + return CONSTRUCTOR_CACHE.computeIfAbsent(constructor, ctor -> computeConstructorHandleAndTypes(ctor, defaultSupplier)); + } + + private static ConstructorHandleAndTypes computeConstructorHandleAndTypes(Constructor constructor, Supplier> defaultSupplier) { + MethodHandle constructorMethodHandle = getConstructorMethodHandle(constructor); if (isGenericInformationLost(constructor)) { - return () -> getFields(constructor) + return new ConstructorHandleAndTypes(constructorMethodHandle, getFields(constructor) .map(Field::getGenericType) - .toArray(Type[]::new); + .collect(toUnmodifiableList())); + } + return new ConstructorHandleAndTypes(constructorMethodHandle, defaultSupplier.get()); + } + + private static MethodHandle getConstructorMethodHandle(Constructor constructor) { + try { + return LOOKUP.unreflectConstructor(constructor).asFixedArity(); + } catch (IllegalAccessException e) { + throw new RuntimeException(e); } - return defaultSupplier; } private static Stream getFields(Constructor constructor) { return Arrays.stream(constructor.getDeclaringClass().getDeclaredFields()) .filter(field -> !Modifier.isStatic(field.getModifiers())); } + + private static class ConstructorHandleAndTypes { + private final MethodHandle constructorHandle; + private final List types; + + ConstructorHandleAndTypes(MethodHandle constructorHandle, List types) { + this.constructorHandle = requireNonNull(constructorHandle, "constructorHandle is null"); + this.types = requireNonNull(types, "types is null"); + } + + public MethodHandle getConstructorHandle() { + return constructorHandle; + } + + public List getTypes() { + return types; + } + } } diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapper.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapper.java index 07c6b5e708..60d8464e17 100644 --- a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapper.java +++ b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorMapper.java @@ -207,14 +207,14 @@ private Optional> createSpecializedRowMapper(StatementContext ctx, List unmatchedColumns) { final int count = factory.getParameterCount(); final Parameter[] parameters = factory.getParameters(); - final Type[] types = factory.getTypes(); + final List types = factory.getTypes(); boolean matchedColumns = false; final List unmatchedParameters = new ArrayList<>(); final List paramData = new ArrayList<>(); for (int i = 0; i < count; i++) { final Parameter parameter = parameters[i]; - final Type parameterType = types[i]; + final Type parameterType = types.get(i); boolean nullable = isNullable(parameter); Nested nested = parameter.getAnnotation(Nested.class); if (nested == null) { diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/InstanceFactory.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/InstanceFactory.java index 2aee37ae77..bac11a4793 100644 --- a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/InstanceFactory.java +++ b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/InstanceFactory.java @@ -18,12 +18,14 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.Arrays; +import java.util.List; import java.util.Objects; import java.util.stream.Stream; import jakarta.annotation.Nullable; import static java.util.Objects.requireNonNull; +import static java.util.stream.Collectors.toUnmodifiableList; abstract class InstanceFactory { private final Executable executable; @@ -44,10 +46,10 @@ Parameter[] getParameters() { return executable.getParameters(); } - Type[] getTypes() { + List getTypes() { return Arrays.stream(getParameters()) .map(Parameter::getParameterizedType) - .toArray(Type[]::new); + .collect(toUnmodifiableList()); } @Nullable From a067a4c3cc03ff1a295c8bfabccd98bc689c58f5 Mon Sep 17 00:00:00 2001 From: Elon Azoulay Date: Sat, 2 Mar 2024 00:57:14 -0800 Subject: [PATCH 3/3] Cache types in ConstructorInstanceFactory --- .../reflect/ConstructorInstanceFactory.java | 41 +++---------------- 1 file changed, 6 insertions(+), 35 deletions(-) diff --git a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java index 04b19d1b66..c498348d46 100644 --- a/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java +++ b/core/src/main/java/org/jdbi/v3/core/mapper/reflect/ConstructorInstanceFactory.java @@ -22,21 +22,16 @@ import java.lang.reflect.Type; import java.util.Arrays; import java.util.List; -import java.util.Map; -import java.util.WeakHashMap; import java.util.function.Supplier; import java.util.stream.Stream; import org.jdbi.v3.core.internal.exceptions.Unchecked; -import static java.util.Collections.synchronizedMap; import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toUnmodifiableList; class ConstructorInstanceFactory extends InstanceFactory { private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup(); - private static final Map CONSTRUCTOR_CACHE = synchronizedMap(new WeakHashMap<>()); - private final Constructor constructor; private final List types; private final MethodHandle constructorHandle; @@ -44,9 +39,8 @@ class ConstructorInstanceFactory extends InstanceFactory { ConstructorInstanceFactory(Constructor constructor) { super(constructor); this.constructor = requireNonNull(constructor, "constructor is null"); - ConstructorHandleAndTypes constructorHandleAndTypes = getConstructorHandleAndTypes(constructor, super::getTypes); - this.types = constructorHandleAndTypes.getTypes(); - this.constructorHandle = constructorHandleAndTypes.getConstructorHandle(); + this.types = extractTypes(constructor, super::getTypes); + this.constructorHandle = getConstructorMethodHandle(constructor); } @Override @@ -88,18 +82,13 @@ private static boolean isGenericInformationLost(Constructor factory) { return lossDetected; } - private static ConstructorHandleAndTypes getConstructorHandleAndTypes(Constructor constructor, Supplier> defaultSupplier) { - return CONSTRUCTOR_CACHE.computeIfAbsent(constructor, ctor -> computeConstructorHandleAndTypes(ctor, defaultSupplier)); - } - - private static ConstructorHandleAndTypes computeConstructorHandleAndTypes(Constructor constructor, Supplier> defaultSupplier) { - MethodHandle constructorMethodHandle = getConstructorMethodHandle(constructor); + private static List extractTypes(Constructor constructor, Supplier> defaultSupplier) { if (isGenericInformationLost(constructor)) { - return new ConstructorHandleAndTypes(constructorMethodHandle, getFields(constructor) + return getFields(constructor) .map(Field::getGenericType) - .collect(toUnmodifiableList())); + .collect(toUnmodifiableList()); } - return new ConstructorHandleAndTypes(constructorMethodHandle, defaultSupplier.get()); + return defaultSupplier.get(); } private static MethodHandle getConstructorMethodHandle(Constructor constructor) { @@ -114,22 +103,4 @@ private static Stream getFields(Constructor constructor) { return Arrays.stream(constructor.getDeclaringClass().getDeclaredFields()) .filter(field -> !Modifier.isStatic(field.getModifiers())); } - - private static class ConstructorHandleAndTypes { - private final MethodHandle constructorHandle; - private final List types; - - ConstructorHandleAndTypes(MethodHandle constructorHandle, List types) { - this.constructorHandle = requireNonNull(constructorHandle, "constructorHandle is null"); - this.types = requireNonNull(types, "types is null"); - } - - public MethodHandle getConstructorHandle() { - return constructorHandle; - } - - public List getTypes() { - return types; - } - } }