From f9dfb6bdc2cf0eba500796893cb28297b6f7dc88 Mon Sep 17 00:00:00 2001 From: Willie Scholtz Date: Mon, 18 Mar 2024 13:57:09 +0100 Subject: [PATCH] #101: Drastically improve performance by only checking if we can create the result once Given that the mapping of the next rows would be identical, this is safe --- .../resultset/DefaultResultSetHandler.java | 38 +++++++++++-------- .../resultset/PendingConstructorCreation.java | 13 +++++-- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/apache/ibatis/executor/resultset/DefaultResultSetHandler.java b/src/main/java/org/apache/ibatis/executor/resultset/DefaultResultSetHandler.java index ef85eac80e1..b43b1c5de47 100644 --- a/src/main/java/org/apache/ibatis/executor/resultset/DefaultResultSetHandler.java +++ b/src/main/java/org/apache/ibatis/executor/resultset/DefaultResultSetHandler.java @@ -1035,12 +1035,17 @@ private String prependPrefix(String columnName, String prefix) { private void handleRowValuesForNestedResultMap(ResultSetWrapper rsw, ResultMap resultMap, ResultHandler resultHandler, RowBounds rowBounds, ResultMapping parentMapping) throws SQLException { final boolean useCollectionConstructorInjection = resultMap.hasResultMapsUsingConstructorCollection(); + boolean verifyPendingCreationResult = true; + PendingConstructorCreation lastHandledCreation = null; + if (useCollectionConstructorInjection) { + verifyPendingCreationPreconditions(parentMapping); + } + final DefaultResultContext resultContext = new DefaultResultContext<>(); ResultSet resultSet = rsw.getResultSet(); skipRows(resultSet, rowBounds); Object rowValue = previousRowValue; - PendingConstructorCreation lastHandledCreation = null; while (shouldProcessMoreRows(resultContext, rowBounds) && !resultSet.isClosed() && resultSet.next()) { final ResultMap discriminatedResultMap = resolveDiscriminatedResultMap(resultSet, resultMap, null); final CacheKey rowKey = createRowKey(discriminatedResultMap, rsw, null); @@ -1051,8 +1056,12 @@ private void handleRowValuesForNestedResultMap(ResultSetWrapper rsw, ResultMap r // issue #577, #542 && #101 if (useCollectionConstructorInjection) { if (foundNewUniqueRow && lastHandledCreation != null) { - createAndStorePendingCreation(resultHandler, parentMapping, resultSet, resultContext, lastHandledCreation); + createAndStorePendingCreation(resultHandler, resultSet, resultContext, lastHandledCreation, + verifyPendingCreationResult); lastHandledCreation = null; + // we only need to verify the first the result for a given result set + // as we can assume the next result will look exactly the same w.r.t its mapping + verifyPendingCreationResult = false; } rowValue = getRowValue(rsw, discriminatedResultMap, rowKey, null, partialObject); @@ -1074,7 +1083,8 @@ private void handleRowValuesForNestedResultMap(ResultSetWrapper rsw, ResultMap r } if (useCollectionConstructorInjection && lastHandledCreation != null) { - createAndStorePendingCreation(resultHandler, parentMapping, resultSet, resultContext, lastHandledCreation); + createAndStorePendingCreation(resultHandler, resultSet, resultContext, lastHandledCreation, + verifyPendingCreationResult); } else if (rowValue != null && mappedStatement.isResultOrdered() && shouldProcessMoreRows(resultContext, rowBounds)) { storeObject(resultHandler, resultContext, rowValue, parentMapping, resultSet); @@ -1221,26 +1231,22 @@ private boolean applyNestedPendingConstructorCreations(ResultSetWrapper rsw, Res return foundValues; } - private void verifyPendingCreationResultOrdered() { + private void verifyPendingCreationPreconditions(ResultMapping parentMapping) { + if (parentMapping != null) { + throw new ExecutorException( + "Cannot construct objects with collections in constructors using multiple result sets yet!"); + } + if (!mappedStatement.isResultOrdered()) { throw new ExecutorException("Cannot reliably construct result if we are not sure the results are ordered " + "so that no new previous rows would occur, set resultOrdered on your mapped statement if you have verified this"); } } - private void createAndStorePendingCreation(ResultHandler resultHandler, ResultMapping parentMapping, - ResultSet resultSet, DefaultResultContext resultContext, PendingConstructorCreation pendingCreation) + private void createAndStorePendingCreation(ResultHandler resultHandler, ResultSet resultSet, + DefaultResultContext resultContext, PendingConstructorCreation pendingCreation, boolean shouldVerify) throws SQLException { - if (parentMapping != null) { - throw new ExecutorException("Not supported in immutable constructor mode yet!"); - } - - verifyPendingCreationResultOrdered(); - - // todo: we can do this verification once per result type in the future - pendingCreation.verifyCanCreate(objectFactory); - final Object result = pendingCreation.create(objectFactory); - + final Object result = pendingCreation.create(objectFactory, shouldVerify); storeObject(resultHandler, resultContext, result, null, resultSet); nestedResultObjects.clear(); } diff --git a/src/main/java/org/apache/ibatis/executor/resultset/PendingConstructorCreation.java b/src/main/java/org/apache/ibatis/executor/resultset/PendingConstructorCreation.java index 14703a04960..9f65fcd3f52 100644 --- a/src/main/java/org/apache/ibatis/executor/resultset/PendingConstructorCreation.java +++ b/src/main/java/org/apache/ibatis/executor/resultset/PendingConstructorCreation.java @@ -107,7 +107,7 @@ void linkCollectionValue(ResultMapping constructorMapping, Object value) { * @param objectFactory * the object factory */ - void verifyCanCreate(ObjectFactory objectFactory) { + private void verifyCanCreate(ObjectFactory objectFactory) { // before we create, we need to get the constructor to be used and verify our types match // since we added to the collection completely unchecked final Constructor resolvedConstructor = objectFactory.resolveConstructor(resultType, constructorArgTypes); @@ -168,10 +168,16 @@ public String toString() { * * @param objectFactory * the object factory + * @param verifyCreate + * should we verify this object can be created, should only be needed once * * @return the new immutable result */ - Object create(ObjectFactory objectFactory) { + Object create(ObjectFactory objectFactory, boolean verifyCreate) { + if (verifyCreate) { + verifyCanCreate(objectFactory); + } + final List newArguments = new ArrayList<>(constructorArgs.size()); for (int i = 0; i < constructorArgs.size(); i++) { final PendingCreationMetaInfo creationMetaInfo = linkedCollectionMetaInfo.get(i); @@ -191,8 +197,7 @@ Object create(ObjectFactory objectFactory) { final List linkedCreations = linkedCreationsByResultMapId.get(resultMapId); for (PendingConstructorCreation linkedCreation : linkedCreations) { - linkedCreation.verifyCanCreate(objectFactory); - emptyCollection.add(linkedCreation.create(objectFactory)); + emptyCollection.add(linkedCreation.create(objectFactory, verifyCreate)); } newArguments.add(emptyCollection);