diff --git a/hibernate-core/src/main/java/org/hibernate/stat/internal/StatisticsImpl.java b/hibernate-core/src/main/java/org/hibernate/stat/internal/StatisticsImpl.java index e2b030f34c..c3f89dea4d 100644 --- a/hibernate-core/src/main/java/org/hibernate/stat/internal/StatisticsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/stat/internal/StatisticsImpl.java @@ -6,8 +6,6 @@ */ package org.hibernate.stat.internal; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.LongAdder; @@ -16,7 +14,6 @@ import org.hibernate.cache.spi.QueryResultsRegion; import org.hibernate.cache.spi.Region; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.internal.CoreMessageLogger; -import org.hibernate.internal.util.collections.ArrayHelper; import org.hibernate.metamodel.model.domain.NavigableRole; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.service.Service; @@ -28,6 +25,7 @@ import static org.hibernate.internal.CoreLogging.messageLogger; * Implementation of {@link org.hibernate.stat.Statistics} based on the {@link java.util.concurrent} package. * * @author Alex Snaps + * @author Sanne Grinovero */ @SuppressWarnings({ "unchecked" }) public class StatisticsImpl implements StatisticsImplementor, Service { @@ -85,21 +83,21 @@ public class StatisticsImpl implements StatisticsImplementor, Service { private final LongAdder optimisticFailureCount = new LongAdder(); - private final ConcurrentMap entityStatsMap = new ConcurrentHashMap(); - private final ConcurrentMap naturalIdQueryStatsMap = new ConcurrentHashMap(); - private final ConcurrentMap collectionStatsMap = new ConcurrentHashMap(); + private final StatsNamedContainer entityStatsMap = new StatsNamedContainer(); + private final StatsNamedContainer naturalIdQueryStatsMap = new StatsNamedContainer(); + private final StatsNamedContainer collectionStatsMap = new StatsNamedContainer(); /** * Keyed by query string */ - private final ConcurrentMap queryStatsMap = new ConcurrentHashMap(); + private final StatsNamedContainer queryStatsMap = new StatsNamedContainer(); /** * Keyed by region name */ - private final ConcurrentMap l2CacheStatsMap = new ConcurrentHashMap<>(); + private final StatsNamedContainer l2CacheStatsMap = new StatsNamedContainer<>(); - private final ConcurrentMap deprecatedNaturalIdStatsMap = new ConcurrentHashMap(); + private final StatsNamedContainer deprecatedNaturalIdStatsMap = new StatsNamedContainer(); @SuppressWarnings({ "UnusedDeclaration" }) @@ -197,7 +195,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { @Override public String[] getEntityNames() { if ( sessionFactory == null ) { - return ArrayHelper.toStringArray( entityStatsMap.keySet() ); + return entityStatsMap.keysAsArray(); } else { return sessionFactory.getMetamodel().getAllEntityNames(); @@ -210,7 +208,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { return null; } - return entityStatsMap.computeIfAbsent( + return entityStatsMap.getOrCompute( entityName, s -> new EntityStatisticsImpl( sessionFactory.getMetamodel().entityPersister( entityName ) ) ); @@ -310,7 +308,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { @Override public String[] getCollectionRoleNames() { if ( sessionFactory == null ) { - return ArrayHelper.toStringArray( collectionStatsMap.keySet() ); + return collectionStatsMap.keysAsArray(); } else { return sessionFactory.getMetamodel().getAllCollectionRoles(); @@ -323,7 +321,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { return null; } - return collectionStatsMap.computeIfAbsent( + return collectionStatsMap.getOrCompute( role, s -> new CollectionStatisticsImpl( sessionFactory.getMetamodel().collectionPersister( role ) ) ); @@ -415,7 +413,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { return null; } - return naturalIdQueryStatsMap.computeIfAbsent( + return naturalIdQueryStatsMap.getOrCompute( rootEntityName, s -> { final EntityPersister entityDescriptor = sessionFactory.getMetamodel().entityPersister( rootEntityName ); @@ -429,8 +427,9 @@ public class StatisticsImpl implements StatisticsImplementor, Service { @Override public DeprecatedNaturalIdCacheStatisticsImpl getNaturalIdCacheStatistics(String regionName) { - return deprecatedNaturalIdStatsMap.computeIfAbsent( - sessionFactory.getCache().unqualifyRegionName( regionName ), + final String key = sessionFactory.getCache().unqualifyRegionName( regionName ); + return deprecatedNaturalIdStatsMap.getOrCompute( + key, unqualifiedRegionName -> new DeprecatedNaturalIdCacheStatisticsImpl( unqualifiedRegionName, sessionFactory.getCache().getNaturalIdAccessesInRegion( unqualifiedRegionName ) @@ -569,7 +568,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { return null; } - return l2CacheStatsMap.computeIfAbsent( + return l2CacheStatsMap.getOrCompute( regionName, s -> { final Region region = sessionFactory.getCache().getRegion( regionName ); @@ -606,7 +605,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { return null; } - return l2CacheStatsMap.computeIfAbsent( + return l2CacheStatsMap.getOrCompute( regionName, s -> new CacheRegionStatisticsImpl( regionAccess.getRegion() ) ); @@ -622,7 +621,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { return null; } - return l2CacheStatsMap.computeIfAbsent( + return l2CacheStatsMap.getOrCompute( regionName, s -> { Region region = sessionFactory.getCache().getRegion( regionName ); @@ -702,12 +701,12 @@ public class StatisticsImpl implements StatisticsImplementor, Service { @Override public String[] getQueries() { - return ArrayHelper.toStringArray( queryStatsMap.keySet() ); + return queryStatsMap.keysAsArray(); } @Override public QueryStatisticsImpl getQueryStatistics(String queryString) { - return queryStatsMap.computeIfAbsent( + return queryStatsMap.getOrCompute( queryString, s -> new QueryStatisticsImpl( queryString ) ); @@ -779,7 +778,7 @@ public class StatisticsImpl implements StatisticsImplementor, Service { } private CacheRegionStatisticsImpl getQueryRegionStats(String regionName) { - return l2CacheStatsMap.computeIfAbsent( + return l2CacheStatsMap.getOrCompute( regionName, s -> new CacheRegionStatisticsImpl( sessionFactory.getCache().getQueryResultsCache( regionName ).getRegion() ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/stat/internal/StatsNamedContainer.java b/hibernate-core/src/main/java/org/hibernate/stat/internal/StatsNamedContainer.java new file mode 100644 index 0000000000..712e475507 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/stat/internal/StatsNamedContainer.java @@ -0,0 +1,71 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.stat.internal; + +import java.util.ArrayList; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; + +/** + * Decorates a ConcurrentHashMap implementation to make sure the methods are being + * used correctly for the purpose of Hibernate's statistics. + * In particular, we do like the semantics of ConcurrentHashMap#computeIfAbsent + * but not how it performs. + * See also: + * - http://jsr166-concurrency.10961.n7.nabble.com/Re-ConcurrentHashMap-computeIfAbsent-td11687.html + * - https://hibernate.atlassian.net/browse/HHH-13527 + * + * @author Sanne Grinovero + */ +final class StatsNamedContainer { + + private final ConcurrentHashMap map = new ConcurrentHashMap(); + + public void clear() { + map.clear(); + } + + /** + * This method is inherently racy and expensive. Only use on non-hot paths, and + * only to get a recent snapshot. + * @return all keys in string form. + */ + public String[] keysAsArray() { + return map.keySet().toArray( new String[0] ); + } + + /** + * Similar semantics as you'd get by invoking {@link java.util.concurrent.ConcurrentMap#computeIfAbsent(Object, Function)}, + * but we check for the key existence first. + * This is technically a redundant check, but it has been shown to perform better when the key existing is very likely, + * as in our case. + * Most notably, the ConcurrentHashMap implementation might block other accesses for the sake of making + * sure the function is invoked at most once: we don't need this guarantee, and prefer to reduce risk of blocking. + */ + public V getOrCompute(final String key, final Function function) { + final V v1 = map.get( key ); + if ( v1 != null ) { + return v1; + } + else { + final V v2 = function.apply( key ); + final V v3 = map.putIfAbsent( key, v2 ); + if ( v3 == null ) { + return v2; + } + else { + return v3; + } + } + } + + public V get(final String key) { + return map.get( key ); + } + +}