Skip to content
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

DefaultResultSetHandler#applyColumnOrderBasedConstructorAutomapping() should call Constructor.getParameterTypes() just once #3113

Closed
whojes-toss opened this issue Mar 19, 2024 · 1 comment · Fixed by #3114
Assignees
Labels
polishing Improve a implementation code or doc without change in current behavior/content
Milestone

Comments

@whojes-toss
Copy link
Contributor

whojes-toss commented Mar 19, 2024

MyBatis version

3.5.14

DefaultResultSetHandler

It's not a bug, but memory resources can be reduced by fixing it i guess

private boolean applyColumnOrderBasedConstructorAutomapping(ResultSetWrapper rsw, List<Class<?>> constructorArgTypes,
List<Object> constructorArgs, Constructor<?> constructor, boolean foundValues) throws SQLException {
for (int i = 0; i < constructor.getParameterTypes().length; i++) {
Class<?> parameterType = constructor.getParameterTypes()[i];
String columnName = rsw.getColumnNames().get(i);
TypeHandler<?> typeHandler = rsw.getTypeHandler(parameterType, columnName);
Object value = typeHandler.getResult(rsw.getResultSet(), columnName);
constructorArgTypes.add(parameterType);
constructorArgs.add(value);
foundValues = value != null || foundValues;
}
return foundValues;
}

In this code block, constructor.getParameterTypes() method is called so much, which clones objects causing memory waste.

private boolean applyColumnOrderBasedConstructorAutomapping(ResultSetWrapper rsw, List<Class<?>> constructorArgTypes, 
     List<Object> constructorArgs, Constructor<?> constructor, boolean foundValues) throws SQLException { 
   for (int i = 0; i < constructor.getParameterTypes().length; i++) { 
     Class<?> parameterType = constructor.getParameterTypes()[i]; 
     String columnName = rsw.getColumnNames().get(i); 
     TypeHandler<?> typeHandler = rsw.getTypeHandler(parameterType, columnName); 
     Object value = typeHandler.getResult(rsw.getResultSet(), columnName); 
     constructorArgTypes.add(parameterType); 
     constructorArgs.add(value); 
     foundValues = value != null || foundValues; 
   } 
   return foundValues; 
}

so i suggest this:

private boolean applyColumnOrderBasedConstructorAutomapping(ResultSetWrapper rsw, List<Class<?>> constructorArgTypes, 
     List<Object> constructorArgs, Constructor<?> constructor, boolean foundValues) throws SQLException { 
   Class<?>[] parameterTypes = constructor.getParameterTypes(); // called once at the beginning
   for (int i = 0; i < parameterTypes.length; i++) { 
     Class<?> parameterType = parameterTypes[i]; 
     String columnName = rsw.getColumnNames().get(i); 
     TypeHandler<?> typeHandler = rsw.getTypeHandler(parameterType, columnName); 
     Object value = typeHandler.getResult(rsw.getResultSet(), columnName); 
     constructorArgTypes.add(parameterType); 
     constructorArgs.add(value); 
     foundValues = value != null || foundValues; 
   } 
   return foundValues; 
}
@harawata
Copy link
Member

Hello @whojes-toss ,

Looks reasonable.
For a proper review, could you submit a pull request?

whojes-toss added a commit to whojes-toss/mybatis-3 that referenced this issue Mar 19, 2024
@harawata harawata changed the title DefaultResultSetHandler memory waste DefaultResultSetHandler#applyColumnOrderBasedConstructorAutomapping() should call Constructor.getParameterTypes() just once Mar 19, 2024
@harawata harawata linked a pull request Mar 19, 2024 that will close this issue
harawata added a commit that referenced this issue Mar 20, 2024
`DefaultResultSetHandler#applyColumnOrderBasedConstructorAutomapping()` should call `Constructor.getParameterTypes()` which internally clones the array just once.

fixes #3113
@harawata harawata self-assigned this Mar 20, 2024
@harawata harawata added the polishing Improve a implementation code or doc without change in current behavior/content label Mar 20, 2024
@harawata harawata added this to the 3.5.16 milestone Mar 20, 2024
sivakiran4u1 pushed a commit to sivakiran4u1/mybatis-3 that referenced this issue Mar 21, 2024
…to inampuds/3.5.15

* '3.5.15' of https://github.com/sivakiran4u1/mybatis-3: (382 commits)
  Update license year
  fix: DefaultResultSetHandler memory waste mybatis#3113
  chore(deps): update dependency org.postgresql:postgresql to v42.7.3
  chore(deps): update log4j2 monorepo to v2.23.1
  chore(deps): update testcontainers-java monorepo to v1.19.7
  chore(deps): update dependency ch.qos.logback:logback-classic to v1.5.3
  chore(deps): update dependency ch.qos.logback:logback-classic to v1.5.2
  chore(deps): update mockito monorepo to v5.11.0
  chore(deps): update dependency ch.qos.logback:logback-classic to v1.5.1
  chore(deps): update dependency org.testcontainers:mysql to v1.19.6
  chore(deps): update log4j2 monorepo to v2.23.0
  fix(deps): update dependency com.microsoft.sqlserver:mssql-jdbc to v12.6.1.jre11
  chore(deps): update dependency org.postgresql:postgresql to v42.7.2
  chore(deps): update dependency ch.qos.logback:logback-classic to v1.5.0
  chore(deps): update byte-buddy.version to v1.14.12
  [git] Update git ignore
  chore(deps): update actions/setup-java action to v4
  chore(deps): update jamesives/github-pages-deploy-action action to v4.5.0
  [GHA] Update sonar.login to sonar.token
  [pom] Remove topSiteURL as defined in parent now
  ...

# Conflicts:
#	pom.xml
#	src/main/java/org/apache/ibatis/mapping/BoundSql.java
#	src/main/java/org/apache/ibatis/scripting/xmltags/XMLScriptBuilder.java
#	src/main/java/org/apache/ibatis/session/Configuration.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polishing Improve a implementation code or doc without change in current behavior/content
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants