From 26f2b8e42af66cd8d636fa3d61ef1f466d40796f Mon Sep 17 00:00:00 2001 From: Andrej Golovnin Date: Mon, 4 Jul 2016 21:42:18 +0200 Subject: [PATCH] HHH-10924 Replace ConcurrentServiceBinding by ConcurrentHashMap --- .../internal/AbstractServiceRegistryImpl.java | 9 +- .../internal/ConcurrentServiceBinding.java | 210 ------------------ .../ConcurrentServiceBindingTest.java | 108 --------- 3 files changed, 6 insertions(+), 321 deletions(-) delete mode 100644 hibernate-core/src/main/java/org/hibernate/service/internal/ConcurrentServiceBinding.java delete mode 100644 hibernate-core/src/test/java/org/hibernate/service/internal/ConcurrentServiceBindingTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java b/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java index 37426a214f..5aaad688af 100644 --- a/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/service/internal/AbstractServiceRegistryImpl.java @@ -11,7 +11,10 @@ import java.util.HashSet; import java.util.List; import java.util.ListIterator; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; import org.hibernate.boot.registry.BootstrapServiceRegistry; import org.hibernate.cfg.Environment; @@ -49,11 +52,11 @@ public abstract class AbstractServiceRegistryImpl private final ServiceRegistryImplementor parent; private final boolean allowCrawling; - private final ConcurrentServiceBinding serviceBindingMap = new ConcurrentServiceBinding(); - private final ConcurrentServiceBinding roleXref = new ConcurrentServiceBinding(); + private final ConcurrentMap serviceBindingMap = new ConcurrentHashMap<>(); + private final ConcurrentMap roleXref = new ConcurrentHashMap<>(); // The services stored in initializedServiceByRole are completely initialized // (i.e., configured, dependencies injected, and started) - private final ConcurrentServiceBinding initializedServiceByRole = new ConcurrentServiceBinding(); + private final ConcurrentMap initializedServiceByRole = new ConcurrentHashMap<>(); // IMPL NOTE : the list used for ordered destruction. Cannot used map above because we need to // iterate it in reverse order which is only available through ListIterator diff --git a/hibernate-core/src/main/java/org/hibernate/service/internal/ConcurrentServiceBinding.java b/hibernate-core/src/main/java/org/hibernate/service/internal/ConcurrentServiceBinding.java deleted file mode 100644 index b49ad6ab53..0000000000 --- a/hibernate-core/src/main/java/org/hibernate/service/internal/ConcurrentServiceBinding.java +++ /dev/null @@ -1,210 +0,0 @@ -/* - * 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.service.internal; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -/** - * Since Service lookup is a very hot operation and essentially it's a read only - * data structure, to achieve thread-safety we can use immutability. - * For our use case we just need reference equality, and the expectation is that a limited - * number of elements will be contained in this custom collection (<32). - * So the following structure is functionally equivalent to an Identity based ConcurrentMap, - * but heavily tuned for reads, at cost of structural reorganization at writes. - * The implementation is a binary tree basing the comparison order on the identityHashCode - * of each key. - * - * @author Sanne Grinovero - */ -public class ConcurrentServiceBinding { - - @SuppressWarnings({ "unchecked", "rawtypes" }) - private static final Node EMPTY_LEAF = new Node( new Entry( 0, null, null ), null, null ); - - @SuppressWarnings("unchecked") - private volatile Node treeRoot = EMPTY_LEAF; - - @SuppressWarnings("unchecked") - public synchronized void clear() { - treeRoot = EMPTY_LEAF; - } - - public synchronized void put(final K key, final V value) { - final int code = hashKey( key ); - final Entry newEntry = new Entry( code, key, value ); - final ArrayList> list = convertToArrayList( treeRoot, key ); - list.add( newEntry ); - Collections.sort( list ); - final int size = list.size(); - @SuppressWarnings("unchecked") - Entry[] array = list.toArray( new Entry[size] ); - treeRoot = treeFromRange( array, 0, size ); - } - - private Node treeFromRange(final Entry[] array, final int minInclusive, final int maxExclusive) { - if ( minInclusive == maxExclusive ) { - return null; - } - //find the midpoint, rounding down to avoid the exclusion range: - int mid = ( minInclusive + maxExclusive ) / 2; - //shift to the right to make sure we won't have left children with the same hash: - while ( mid > minInclusive && array[mid].hash == array[mid-1].hash ) { - mid--; - } - return new Node( array[mid], treeFromRange( array, minInclusive, mid ), treeFromRange( array, mid + 1, maxExclusive ) ); - } - - public V get(final K key) { - final int hash = hashKey( key ); - final Node root = treeRoot; - return root.get( key, hash ); - } - - protected int hashKey(final K key) { - return System.identityHashCode( key ); - } - - public Iterable values() { - @SuppressWarnings("rawtypes") - ArrayList list = new ArrayList(); - treeRoot.collectAllValuesInto( list ); - return list; - } - - private final ArrayList> convertToArrayList(final Node treeRoot, K exceptKey) { - @SuppressWarnings("rawtypes") - ArrayList> list = new ArrayList(); - if ( treeRoot != EMPTY_LEAF ) { - treeRoot.collectAllEntriesInto( list, exceptKey ); - } - return list; - } - - private static final class Entry implements Comparable> { - - private final int hash; - private final K key; - private final V value; - - Entry(int keyHashCode, K key, V value) { - this.hash = keyHashCode; - this.key = key; - this.value = value; - } - - @Override - public int compareTo(Entry o) { - //Sorting by the identity hashcode - //Note: this class has a natural ordering that is inconsistent with equals. - return ( hash < o.hash ) ? -1 : ( (hash == o.hash) ? 0 : 1 ); - } - - @Override - public int hashCode() { - return hash; - } - - @Override - @SuppressWarnings({"unchecked", "EqualsWhichDoesntCheckParameterClass"}) - public boolean equals(Object obj) { - //A ClassCastException is really not expected here, - //as it's an internal private class, - //so just let it happen as a form of assertion. - final Entry other = (Entry)obj; - //Reference equality on the key only! - return other != null && other.key == this.key; - } - - @Override - public String toString() { - return "<" + key + ", " + value + ">"; - } - } - - private static final class Node { - - private final Entry entry; - private final Node left; - private final Node right; - - Node(Entry entry, Node left, Node right) { - this.entry = entry; - this.left = left; - this.right = right; - } - - public V get(final K key, final int hash) { - if ( entry.key == key ) { - return entry.value; - } - //Note that same-hashcode childs need to be on the right - //as we don't test for equality, nor want to chase both - //branches: - else if ( hash < this.entry.hash ) { - return left == null ? null : left.get( key, hash ); - } - else { - return right == null ? null : right.get( key, hash ); - } - } - - public void collectAllEntriesInto(final List> list, final K exceptKey) { - if ( entry != null && exceptKey != entry.key ) { - list.add( entry ); - } - if ( left != null ) { - left.collectAllEntriesInto( list, exceptKey ); - } - if ( right != null ) { - right.collectAllEntriesInto( list, exceptKey ); - } - } - - public void collectAllValuesInto(final List list) { - if ( entry != null && entry.value != null ) { - list.add( entry.value ); - } - if ( left != null ) { - left.collectAllValuesInto( list ); - } - if ( right != null ) { - right.collectAllValuesInto( list ); - } - } - - /** - * Helper to visualize the tree via toString - */ - private void renderToStringBuilder(final StringBuilder sb, final int indent) { - sb.append( entry ); - appendIndented( sb, indent, "L-> ", left ); - appendIndented( sb, indent, "R-> ", right ); - } - - private void appendIndented(final StringBuilder sb, final int indent, final String label, Node node) { - if ( node == null ) { - return; - } - sb.append( "\n" ); - for ( int i = 0; i < indent; i++ ) { - sb.append( "\t" ); - } - sb.append( label ); - node.renderToStringBuilder( sb, indent + 1 ); - } - - @Override - public String toString() { - StringBuilder sb = new StringBuilder(); - renderToStringBuilder( sb, 0 ); - return sb.toString(); - } - } - -} diff --git a/hibernate-core/src/test/java/org/hibernate/service/internal/ConcurrentServiceBindingTest.java b/hibernate-core/src/test/java/org/hibernate/service/internal/ConcurrentServiceBindingTest.java deleted file mode 100644 index aaa8407f07..0000000000 --- a/hibernate-core/src/test/java/org/hibernate/service/internal/ConcurrentServiceBindingTest.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * 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.service.internal; - -import java.util.HashMap; -import java.util.HashSet; -import java.util.Set; - -import org.hibernate.engine.jdbc.connections.internal.ConnectionProviderInitiator; -import org.hibernate.service.spi.ServiceBinding; -import org.hibernate.testing.TestForIssue; -import org.junit.Assert; -import org.junit.Test; - -/** - * Unit Test for ConcurrentServiceBinding - * - * @author Sanne Grinovero - */ -@TestForIssue(jiraKey="HHH-8947") -public class ConcurrentServiceBindingTest { - - private Class[] testTypes = new Class[]{ String.class, Integer.class, ServiceBinding.class, ConnectionProviderInitiator.class, HashMap.class, - ConcurrentServiceBindingTest.class, Long.class, Test.class, Set.class, HashSet.class }; - - @Test - public void normalImplementationTest() { - final ConcurrentServiceBinding binder = new ConcurrentServiceBinding(); - verifyBehaviour( binder ); - } - - @Test - public void allKeysCollisions() { - final ConcurrentServiceBinding binder = new ConcurrentServiceBinding() { - protected int hashKey(final Class key) { - return 15; - } - }; - verifyBehaviour( binder ); - } - - @Test - public void someKeysCollisions() { - final Set collidingClasses = new HashSet(); - collidingClasses.add( String.class ); - collidingClasses.add( ServiceBinding.class ); - collidingClasses.add( ConnectionProviderInitiator.class ); - final Set classedWhichHit = new HashSet(); - final ConcurrentServiceBinding binder = new ConcurrentServiceBinding() { - protected int hashKey(final Class key) { - if ( collidingClasses.contains( key ) ) { - classedWhichHit.add( key ); - return 15; - } - else { - return System.identityHashCode( key ); - } - } - }; - verifyBehaviour( binder ); - Assert.assertEquals( 3, classedWhichHit.size() );//to verify the test is being applied as expected - } - - private void verifyBehaviour(ConcurrentServiceBinding binder) { - isEmpty( binder ); - HashSet addedTypes = new HashSet(); - for ( Class newtype : testTypes ) { - addedTypes.add( newtype ); - binder.put( newtype, newtype.toString() ); - containsExactly( binder, addedTypes ); - } - binder.clear(); - isEmpty( binder ); - } - - private void containsExactly(ConcurrentServiceBinding binder, HashSet addedTypes) { - for ( Class knownType : addedTypes ) { - final String value = binder.get( knownType ); - Assert.assertNotNull( value ); - Assert.assertEquals( knownType.toString(), value ); - int countElements = 0; - boolean present = false; - for ( String each : binder.values() ) { - countElements++; - if ( each.equals( knownType.toString() ) ) { - Assert.assertFalse( "should have been unique", present ); - present = true; - } - } - Assert.assertEquals( addedTypes.size(), countElements ); - Assert.assertTrue( present ); - } - } - - private void isEmpty(ConcurrentServiceBinding binder) { - for ( String value : binder.values() ) { - Assert.fail( "Expected it to be empty" ); - } - for ( Class type : testTypes ) { - Assert.assertNull( binder.get( type ) ); - } - } - -}