Mitigate performance regressions #1772
Conversation
We now create RowMapper/ResultSetExtractor instances only once if they are considered static configuration. A configured RowMapper ref/class is always static. A configured ResultSetExtractor ref/class is static when the extractor does not accept a RowMapper or if the RowMapper is configured. Default mappers or projection-specific ones require ResultSetExtractor recreation of the ResultSetExtractor is configured as Class. Also, reuse TypeInformation for RelationalParameter instead of recreating it.
Reuse TypeInformation as much as possible to avoid Class -> TypeInformation conversion. Introduce LRU cache for DefaultAggregatePath to avoid PersistentPropertyPath lookups. Introduce best-effort quoted cache for SqlIdentifier to avoid excessive string object creation.
| @@ -192,7 +192,7 @@ private JdbcQueryExecution<?> getJdbcQueryExecution(@Nullable ResultSetExtractor | |||
| if (getQueryMethod().isModifyingQuery()) { | |||
| return createModifyingQueryExecutor(); | |||
| } else { | |||
| return createReadingQueryExecution(extractor, rowMapper); | |||
| return createReadingQueryExecution(extractor, () -> rowMapper); | |||
There was a problem hiding this comment.
Shouldn't we replace the parameter rowMapper with a Supplier? This method is called a single time and the rowMapper is constructed right before that usage so converting it to a Supplier should be fairly simple.
| this.sql = processing; | ||
| return sqlName; | ||
| String normalized = processing.standardizeLetterCase(name); | ||
| this.sqlName = sqlName = new CachedSqlName(processing, quoted ? processing.quote(normalized) : normalized); |
There was a problem hiding this comment.
This looks really weird to my old Java eyes.
And I don't even understand why you have the local variable sqlName in the first place.
If it isn't a left over from editing, we should explain it with a comment.
There was a problem hiding this comment.
The light-weight cache is volatile. Any comparison to the field are only valid for the duration of a single instruction. Therefore, we copy the cached field value into a local variable and do our comparisons there. After we discover that we should create a new cached variant, we create CachedSqlName, assign it into the local variable and the volatile field in a single line.
| @@ -57,7 +58,7 @@ public abstract class Jsr310TimestampBasedConverters { | |||
| */ | |||
| public static Collection<Converter<?, ?>> getConvertersToRegister() { | |||
|
|
|||
| List<Converter<?, ?>> converters = new ArrayList<>(8); | |||
| List<Converter<?, ?>> converters = new ArrayList<>(28); | |||
There was a problem hiding this comment.
Meh, that was a leftover from the code below (TODO). We should discuss that one.
|
Merged. |
|
Thanks for working on this, much appreciated. |
See #1721