From dfa26e0b5c8d58792b13813b7215bf85dc24b10d Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Tue, 27 Jun 2023 20:47:00 +0100 Subject: [PATCH] HHH-16815 Improvements in SqmInterpretationsKey --- .../main/java/org/hibernate/LockOptions.java | 18 ++++- .../sqm/internal/SqmInterpretationsKey.java | 66 +++++++++++++------ 2 files changed, 62 insertions(+), 22 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/LockOptions.java b/hibernate-core/src/main/java/org/hibernate/LockOptions.java index d94c09d1fc..eefaffa735 100644 --- a/hibernate-core/src/main/java/org/hibernate/LockOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/LockOptions.java @@ -512,7 +512,7 @@ public class LockOptions implements Serializable { } /** - * Make a copy. + * Make a copy. The new copy will be mutable even if the original wasn't. * * @return The copy */ @@ -522,6 +522,22 @@ public class LockOptions implements Serializable { return copy; } + /** + * Make a copy, unless this is an immutable instance. + * + * @return The copy, or this if it was immutable. + */ + public LockOptions makeDefensiveCopy() { + if ( immutable ) { + return this; + } + else { + final LockOptions copy = new LockOptions(); + copy( this, copy ); + return copy; + } + } + /** * Copy the given lock options into this instance, * merging the alias-specific lock modes. diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java index 989d77486a..a87cf243a3 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java @@ -7,8 +7,10 @@ package org.hibernate.query.sqm.internal; import java.util.Collection; -import java.util.HashSet; +import java.util.Iterator; import java.util.List; +import java.util.Objects; +import java.util.Set; import java.util.function.Supplier; import jakarta.persistence.criteria.Order; @@ -26,7 +28,7 @@ import static org.hibernate.query.spi.AbstractSelectionQuery.CRITERIA_HQL_STRING /** * @author Steve Ebersole */ -public class SqmInterpretationsKey implements QueryInterpretationCache.Key { +public final class SqmInterpretationsKey implements QueryInterpretationCache.Key { public interface CacheabilityInfluencers { boolean isQueryPlanCacheable(); String getQueryString(); @@ -43,16 +45,18 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { public static SqmInterpretationsKey createInterpretationsKey(InterpretationsKeySource keySource) { if ( isCacheable ( keySource ) ) { + final Object query = CRITERIA_HQL_STRING.equals( keySource.getQueryString() ) + ? keySource.getSqmStatement() + : keySource.getQueryString(); return new SqmInterpretationsKey( - CRITERIA_HQL_STRING.equals( keySource.getQueryString() ) - ? keySource.getSqmStatement() - : keySource.getQueryString(), + query, + query.hashCode(), keySource.getResultType(), keySource.getOrder(), keySource.getQueryOptions().getLockOptions(), keySource.getQueryOptions().getTupleTransformer(), keySource.getQueryOptions().getResultListTransformer(), - new HashSet<>( keySource.getLoadQueryInfluencers().getEnabledFetchProfileNames() ) + memoryEfficientDefensiveSetCopy( keySource.getLoadQueryInfluencers().getEnabledFetchProfileNames() ) ); } else { @@ -60,6 +64,25 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { } } + private static Collection memoryEfficientDefensiveSetCopy(final Set set) { + if ( set == null ) { + return null; + } + else { + switch ( set.size() ) { + case 0: + return null; + case 1: + return Set.of( set.iterator().next() ); + case 2: + final Iterator iterator = set.iterator(); + return Set.of( iterator.next(), iterator.next() ); + default: + return Set.copyOf( set ); + } + } + } + private static boolean isCacheable(InterpretationsKeySource keySource) { assert keySource.getQueryOptions().getAppliedGraph() != null; @@ -95,9 +118,11 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { private final TupleTransformer tupleTransformer; private final ResultListTransformer resultListTransformer; private final Collection enabledFetchProfiles; + private final int hashcode; private SqmInterpretationsKey( Object query, + int hash, Class resultType, List order, LockOptions lockOptions, @@ -105,6 +130,7 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { ResultListTransformer resultListTransformer, Collection enabledFetchProfiles) { this.query = query; + this.hashcode = hash; this.resultType = resultType; this.order = order; this.lockOptions = lockOptions; @@ -117,10 +143,11 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { public QueryInterpretationCache.Key prepareForStore() { return new SqmInterpretationsKey( query, + hashcode, resultType, - // Since lock options are mutable, we need a copy for the cache key order, - lockOptions.makeCopy(), + // Since lock options might be mutable, we need a copy for the cache key + lockOptions.makeDefensiveCopy(), tupleTransformer, resultListTransformer, enabledFetchProfiles @@ -137,26 +164,23 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { if ( this == o ) { return true; } - if ( o == null || getClass() != o.getClass() ) { + if ( o == null || SqmInterpretationsKey.class != o.getClass() ) { return false; } final SqmInterpretationsKey that = (SqmInterpretationsKey) o; - return query.equals( that.query ) - && areEqual( resultType, that.resultType ) - && areEqual( order, that.order ) - && areEqual( lockOptions, that.lockOptions ) - && areEqual( tupleTransformer, that.tupleTransformer ) - && areEqual( resultListTransformer, that.resultListTransformer ) - && areEqual( enabledFetchProfiles, that.enabledFetchProfiles ); - } - - private boolean areEqual(T o1, T o2) { - return o1 == null ? o2 == null : o1.equals(o2); + return this.hashcode == o.hashCode() //check this first as some other checks are expensive + && query.equals( that.query ) + && Objects.equals( resultType, that.resultType ) + && Objects.equals( order, that.order ) + && Objects.equals( lockOptions, that.lockOptions ) + && Objects.equals( tupleTransformer, that.tupleTransformer ) + && Objects.equals( resultListTransformer, that.resultListTransformer ) + && Objects.equals( enabledFetchProfiles, that.enabledFetchProfiles ); } @Override public int hashCode() { - return query.hashCode(); + return hashcode; } }