From 4c8bd11610988980e9e59aa5d5d5ff887e37f49a Mon Sep 17 00:00:00 2001 From: evanchooly Date: Mon, 19 Oct 2026 22:11:45 -0400 Subject: [PATCH] Set values directly for no-arg constructors (plan item 3.2) Entities with a no-arg constructor, which bestConstructor picks whenever one exists, went through ConstructorCreator buffering every decoded property in two lists and replaying them into the instance at the end. With no constructor parameters there is nothing to wait for, so the creator now constructs on the first value and sets each one through model.setValue as it arrives, which keeps Conversions. The pending lists are allocated only when a partial constructor needs them, a no-arg creator shares one empty parameter array, and the unnamed-parameter check uses a precompiled pattern. NoArgCreator and the factory branch that created it were unreachable: bestConstructor already returns any no-arg constructor. Both are removed, along with the branch's log line. --- .../mapping/InstanceCreatorFactoryImpl.java | 18 ++---- .../dev/morphia/mapping/NoArgCreator.java | 57 ------------------- .../mapping/internal/ConstructorCreator.java | 29 ++++++++-- .../internal/ConstructorCreatorTest.java | 16 ++++++ design/mapping-game-plan.md | 2 +- 5 files changed, 44 insertions(+), 78 deletions(-) delete mode 100644 core/src/main/java/dev/morphia/mapping/NoArgCreator.java diff --git a/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java b/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java index c5d25a639e5..aee96dbf53a 100644 --- a/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java +++ b/core/src/main/java/dev/morphia/mapping/InstanceCreatorFactoryImpl.java @@ -12,17 +12,12 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** * @hidden * @morphia.internal */ @MorphiaInternal public class InstanceCreatorFactoryImpl implements InstanceCreatorFactory { - private static final Logger LOG = LoggerFactory.getLogger(InstanceCreatorFactoryImpl.class); - private final EntityModel model; private Function creator; @@ -45,15 +40,10 @@ public MorphiaInstanceCreator create(Conversions conversions) { ConstructorCreator.Plan plan = new ConstructorCreator.Plan(model, constructor); creator = (c) -> new ConstructorCreator(plan, c); } else { - LOG.info("using old creator approach: " + model.getType().getName()); - try { - Constructor declared = model.getType().getDeclaredConstructor(); - creator = (c) -> new NoArgCreator(declared); - } catch (NoSuchMethodException e) { - Constructor full = ConstructorCreator.getFullConstructor(model); - ConstructorCreator.Plan plan = new ConstructorCreator.Plan(model, full); - creator = (c) -> new ConstructorCreator(plan, c); - } + // bestConstructor already picks a no-arg constructor when there is one + Constructor full = ConstructorCreator.getFullConstructor(model); + ConstructorCreator.Plan plan = new ConstructorCreator.Plan(model, full); + creator = (c) -> new ConstructorCreator(plan, c); } } diff --git a/core/src/main/java/dev/morphia/mapping/NoArgCreator.java b/core/src/main/java/dev/morphia/mapping/NoArgCreator.java deleted file mode 100644 index 82aefcd86cf..00000000000 --- a/core/src/main/java/dev/morphia/mapping/NoArgCreator.java +++ /dev/null @@ -1,57 +0,0 @@ -package dev.morphia.mapping; - -import java.lang.reflect.Constructor; - -import dev.morphia.annotations.internal.MorphiaInternal; -import dev.morphia.mapping.codec.MorphiaInstanceCreator; -import dev.morphia.mapping.codec.pojo.PropertyModel; -import dev.morphia.sofia.Sofia; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - -/** - * @hidden - * @morphia.internal - */ -@MorphiaInternal -public class NoArgCreator implements MorphiaInstanceCreator { - private Object instance; - private final Constructor noArgsConstructor; - - /** - * Creates the creator - * - * @param noArgsConstructor the constructor - */ - @SuppressFBWarnings("EI_EXPOSE_REP2") - public NoArgCreator(Constructor noArgsConstructor) { - this.noArgsConstructor = noArgsConstructor; - this.noArgsConstructor.setAccessible(true); - } - - @Override - public void set(Object value, PropertyModel model) { - model.getAccessor().set(instance(), value); - } - - private Object instance() { - if (instance == null) { - try { - instance = noArgsConstructor.newInstance(); - } catch (ReflectiveOperationException e) { - throw new MappingException(Sofia.instantiationProblem(noArgsConstructor.getDeclaringClass().getName(), e.getMessage()), e); - } - } - return instance; - } - - @Override - public Object getInstance() { - return instance(); - } - - @Override - public boolean isEagerInstanceSafe() { - return true; - } -} diff --git a/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java b/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java index be500cfc365..388b8c078cf 100644 --- a/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java +++ b/core/src/main/java/dev/morphia/mapping/internal/ConstructorCreator.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.Objects; import java.util.TreeMap; +import java.util.regex.Pattern; import com.mongodb.lang.Nullable; @@ -39,11 +40,17 @@ */ @MorphiaInternal public class ConstructorCreator implements MorphiaInstanceCreator { + private static final Pattern UNNAMED_PARAMETER = Pattern.compile("arg[0-9]+"); + private static final Object[] NO_PARAMETERS = new Object[0]; + private final Plan plan; private final Conversions conversions; private final Object[] parameters; - private final List pendingModels = new ArrayList<>(); - private final List pendingValues = new ArrayList<>(); + // properties the constructor doesn't take, set once it has run; allocated only if a value arrives before then + @Nullable + private List pendingModels; + @Nullable + private List pendingValues; private Object instance; /** @@ -63,7 +70,7 @@ public ConstructorCreator(EntityModel model, Constructor constructor, Convers public ConstructorCreator(Plan plan, Conversions conversions) { this.plan = plan; this.conversions = conversions; - this.parameters = plan.zeroValues.clone(); + this.parameters = plan.types.length == 0 ? NO_PARAMETERS : plan.zeroValues.clone(); } /** @@ -93,7 +100,7 @@ public Plan(EntityModel model, Constructor constructor) { types[i] = parameter.getType(); zeroValues[i] = zeroValue(parameter); String name = getParameterName(parameter); - if (name.matches("arg[0-9]+")) { + if (UNNAMED_PARAMETER.matcher(name).matches()) { throw new MappingException(Sofia.unnamedConstructorParameter(model.getType().getName())); } if (positions.put(name, i) != null) { @@ -138,8 +145,10 @@ public Object getInstance() { if (instance == null) { try { instance = plan.constructor.newInstance(parameters); - for (int i = 0; i < pendingModels.size(); i++) { - pendingModels.get(i).setValue(instance, pendingValues.get(i)); + if (pendingModels != null) { + for (int i = 0; i < pendingModels.size(); i++) { + pendingModels.get(i).setValue(instance, pendingValues.get(i)); + } } } catch (Exception e) { throw new MappingException(Sofia.cannotInstantiate(plan.model.getType().getName(), e.getMessage()), e); @@ -193,6 +202,10 @@ private static boolean namesMatchProperties(EntityModel model, Constructor co public void set(@Nullable Object value, PropertyModel model) { if (instance != null) { model.setValue(instance, value); + } else if (plan.types.length == 0) { + // a no-arg constructor needs none of the values, so construct now and set each value as it arrives instead + // of buffering them all + model.setValue(getInstance(), value); } else { Integer position = plan.positions.get(model.getName()); if (position != null) { @@ -200,6 +213,10 @@ public void set(@Nullable Object value, PropertyModel model) { } else { // only properties the constructor doesn't take need setting after construction. re-setting the others would // overwrite whatever the constructor did with the value and, for final fields, write them reflectively (JEP 500). + if (pendingModels == null) { + pendingModels = new ArrayList<>(); + pendingValues = new ArrayList<>(); + } pendingModels.add(model); pendingValues.add(value); } diff --git a/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java b/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java index b05bc8f798c..6e4cd611eaa 100644 --- a/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java +++ b/core/src/test/java/dev/morphia/test/mapping/internal/ConstructorCreatorTest.java @@ -95,6 +95,22 @@ public void testBestConstructor() { Assertions.assertEquals(0, constructor.getParameterCount()); } + @Test + public void noArgConstructorSetsValuesAsTheyArrive() { + EntityModel model = getMapper().map(Default.class).get(0); + MorphiaInstanceCreator creator = model.getInstanceCreator(getMapper().getConversions()); + Assertions.assertTrue(creator.isEagerInstanceSafe()); + + creator.set("named", model.getProperty("name")); + Default instance = (Default) creator.getInstance(); + Assertions.assertEquals("named", instance.name); + + // a value that needs converting still goes through Conversions on this path + creator.set(42.0, model.getProperty("count")); + Assertions.assertSame(instance, creator.getInstance()); + Assertions.assertEquals(42, instance.count); + } + @Test public void creatorsDoNotShareState() { EntityModel model = getMapper().map(Address.class).get(0); diff --git a/design/mapping-game-plan.md b/design/mapping-game-plan.md index 9d4363415d6..32c5c9985e0 100644 --- a/design/mapping-game-plan.md +++ b/design/mapping-game-plan.md @@ -210,7 +210,7 @@ JMH numbers on both mappers. - Optional: send a known-rejected type straight to the fallback, avoiding a throw and stack trace for each drifted field. -### 3.2 No-arg creator fast path (#2, A2/B4) +### ~~3.2 No-arg creator fast path (#2, A2/B4)~~ - When the chosen constructor has no parameters, construct the instance first and set values through `model.setValue`, which keeps `Conversions`. Allocate pending lists lazily. - Delete the dead `NoArgCreator` branch and its `LOG.info`. Delete `NoArgCreator` itself, which also skips