From 324d5536e4d4108515f87e76183dd2a5270f44b2 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Fri, 7 Feb 2014 18:56:57 +0000 Subject: [PATCH] HHH-8947 Optimize performance of ServiceRegistry for intense lookup, small contents and almost no writes --- .../internal/AbstractServiceRegistryImpl.java | 2 +- .../internal/ConcurrentServiceBinding.java | 221 ++++++++++++++++++ .../ConcurrentServiceBindingTest.java | 125 ++++++++++ 3 files changed, 347 insertions(+), 1 deletion(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/service/internal/ConcurrentServiceBinding.java create 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 0970dcb3aa..7850306a4a 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 @@ -63,7 +63,7 @@ public abstract class AbstractServiceRegistryImpl private final ServiceRegistryImplementor parent; private final boolean allowCrawling; - private final ConcurrentHashMap serviceBindingMap = CollectionHelper.concurrentMap( 20 ); + private final ConcurrentServiceBinding serviceBindingMap = new ConcurrentServiceBinding(); private ConcurrentHashMap roleXref; // IMPL NOTE : the list used for ordered destruction. Cannot used map above because we need to 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 new file mode 100644 index 0000000000..aef9b5eae6 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/service/internal/ConcurrentServiceBinding.java @@ -0,0 +1,221 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +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 threadsafety 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 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.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 new file mode 100644 index 0000000000..56e9dd5867 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/service/internal/ConcurrentServiceBindingTest.java @@ -0,0 +1,125 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +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 ) ); + } + } + +}