HHH-5217 - Minimize double sequence value reads in PooledOptimizer

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@19478 1b8cb986-b30d-0410-93ca-fae66ebed9b2

core/src/main/java/org/hibernate/id/enhanced/OptimizerFactory.java

@@ -44,10 +44,40 @@ public class OptimizerFactory {
 
 	public static final String NONE = "none";
 	public static final String HILO = "hilo";
+	public static final String LEGACY_HILO = "legacy-hilo";
 	public static final String POOL = "pooled";
 
 	private static Class[] CTOR_SIG = new Class[] { Class.class, int.class };
 
+	/**
+	 * Marker interface for optimizer which wish to know the user-specified initial value.
+	 * <p/>
+	 * Used instead of constructor injection since that is already a public understanding and
+	 * because not all optimizers care.
+	 */
+	public static interface InitialValueAwareOptimizer {
+		/**
+		 * Reports the user specified initial value to the optimizer.
+		 * <p/>
+		 * <tt>-1</tt> is used to indicate that the user did not specify.
+		 *
+		 * @param initialValue The initial value specified by the user, or <tt>-1</tt> to indicate that the
+		 * user did not specify.
+		 */
+		public void injectInitialValue(long initialValue);
+	}
+
+	/**
+	 * Builds an optimizer
+	 *
+	 * @param type The optimizer type, either a short-hand name or the {@link Optimizer} class name.
+	 * @param returnClass The generated value java type
+	 * @param incrementSize The increment size.
+	 *
+	 * @return The built optimizer
+	 *
+	 * @deprecated Use {@link #buildOptimizer(String, Class, int, long)} instead
+	 */
 	@SuppressWarnings({ "UnnecessaryBoxing" })
 	public static Optimizer buildOptimizer(String type, Class returnClass, int incrementSize) {
 		String optimizerClassName;
@@ -57,6 +87,9 @@ public class OptimizerFactory {
 		else if ( HILO.equals( type ) ) {
 			optimizerClassName = HiLoOptimizer.class.getName();
 		}
+		else if ( LEGACY_HILO.equals( type ) ) {
+			optimizerClassName = LegacyHiLoAlgorithmOptimizer.class.getName();
+		}
 		else if ( POOL.equals( type ) ) {
 			optimizerClassName = PooledOptimizer.class.getName();
 		}
@@ -70,13 +103,33 @@ public class OptimizerFactory {
 			return ( Optimizer ) ctor.newInstance( returnClass, Integer.valueOf( incrementSize ) );
 		}
 		catch( Throwable ignore ) {
-			// intentionally empty
+			log.warn( "Unable to instantiate specified optimizer [{}], falling back to noop", type );
 		}
 
 		// the default...
 		return new NoopOptimizer( returnClass, incrementSize );
 	}
 
+
+	/**
+	 * Builds an optimizer
+	 *
+	 * @param type The optimizer type, either a short-hand name or the {@link Optimizer} class name.
+	 * @param returnClass The generated value java type
+	 * @param incrementSize The increment size.
+	 * @param explicitInitialValue The user supplied initial-value (-1 indicates the user did not specify).
+	 *
+	 * @return The built optimizer
+	 */
+	@SuppressWarnings({ "UnnecessaryBoxing", "deprecation" })
+	public static Optimizer buildOptimizer(String type, Class returnClass, int incrementSize, long explicitInitialValue) {
+		final Optimizer optimizer = buildOptimizer( type, returnClass, incrementSize );
+		if ( InitialValueAwareOptimizer.class.isInstance( optimizer ) ) {
+			( (InitialValueAwareOptimizer) optimizer ).injectInitialValue( explicitInitialValue );
+		}
+		return optimizer;
+	}
+
 	/**
 	 * Common support for optimizer implementations.
 	 */
@@ -335,9 +388,10 @@ public class OptimizerFactory {
 	 * {@link HiLoOptimizer} except that here the bucket ranges are actually
 	 * encoded into the database structures.
 	 */
-	public static class PooledOptimizer extends OptimizerSupport {
+	public static class PooledOptimizer extends OptimizerSupport implements InitialValueAwareOptimizer {
 		private IntegralDataTypeHolder hiValue;
 		private IntegralDataTypeHolder value;
+		private long initialValue = -1;
 
 		public PooledOptimizer(Class returnClass, int incrementSize) {
 			super( returnClass, incrementSize );
@@ -362,8 +416,15 @@ public class OptimizerFactory {
 					// we are using a sequence...
 					log.info( "pooled optimizer source reported [" + value + "] as the initial value; use of 1 or greater highly recommended" );
 				}
+				if ( ( initialValue == -1 && value.lt( incrementSize ) ) || value.eq( initialValue ) ) {
+					// the call to obtain next-value just gave us the initialValue
 					hiValue = callback.getNextValue();
 				}
+				else {
+					hiValue = value;
+					value = hiValue.copy().subtract( incrementSize );
+				}
+			}
 			else if ( ! hiValue.gt( value ) ) {
 				hiValue = callback.getNextValue();
 				value = hiValue.copy().subtract( incrementSize );
@@ -395,5 +456,12 @@ public class OptimizerFactory {
 		public IntegralDataTypeHolder getLastValue() {
 			return value.copy().decrement();
 		}
+
+		/**
+		 * {@inheritDoc}
+		 */
+		public void injectInitialValue(long initialValue) {
+			this.initialValue = initialValue;
+		}
 	}
 }

core/src/main/java/org/hibernate/id/enhanced/SequenceStyleGenerator.java

@@ -187,8 +187,12 @@ public class SequenceStyleGenerator implements PersistentIdentifierGenerator, Co
 				initialValue,
 				incrementSize
 		);
-
+		this.optimizer = OptimizerFactory.buildOptimizer(
-		this.optimizer = OptimizerFactory.buildOptimizer( optimizationStrategy, identifierType.getReturnedClass(), incrementSize );
+				optimizationStrategy,
+				identifierType.getReturnedClass(),
+				incrementSize,
+				PropertiesHelper.getInt( INITIAL_PARAM, params, -1 )
+		);
 		this.databaseStructure.prepare( optimizer );
 	}
 
@@ -305,7 +309,9 @@ public class SequenceStyleGenerator implements PersistentIdentifierGenerator, Co
 	 * @param forceTableUse Should a table be used even if the dialect supports sequences?
 	 * @param sequenceName The name to use for the sequence or table.
 	 * @param initialValue The initial value.
-	 * @param incrementSize the increment size to use (after any adjustments).       @return The db structure representation
+	 * @param incrementSize the increment size to use (after any adjustments).
+	 *
+	 * @return An abstraction for the actual database structure in use (table vs. sequence).
 	 */
 	protected DatabaseStructure buildDatabaseStructure(
 			Type type,

core/src/main/java/org/hibernate/id/enhanced/TableGenerator.java

@@ -303,9 +303,14 @@ public class TableGenerator extends TransactionHelper implements PersistentIdent
 		this.updateQuery = buildUpdateQuery();
 		this.insertQuery = buildInsertQuery();
 
-		String defOptStrategy = incrementSize <= 1 ? OptimizerFactory.NONE : OptimizerFactory.POOL;
+		final String defOptStrategy = incrementSize <= 1 ? OptimizerFactory.NONE : OptimizerFactory.POOL;
-		String optimizationStrategy = PropertiesHelper.getString( OPT_PARAM, params, defOptStrategy );
+		final String optimizationStrategy = PropertiesHelper.getString( OPT_PARAM, params, defOptStrategy );
-		optimizer = OptimizerFactory.buildOptimizer( optimizationStrategy, identifierType.getReturnedClass(), incrementSize );
+		optimizer = OptimizerFactory.buildOptimizer(
+				optimizationStrategy,
+				identifierType.getReturnedClass(),
+				incrementSize,
+				PropertiesHelper.getInt( INITIAL_PARAM, params, -1 )
+		);
 	}
 
 	/**
@@ -430,8 +435,8 @@ public class TableGenerator extends TransactionHelper implements PersistentIdent
 		String query = "select " + StringHelper.qualify( alias, valueColumnName ) +
 				" from " + tableName + ' ' + alias +
 				" where " + StringHelper.qualify( alias, segmentColumnName ) + "=?";
-		LockOptions lockOptions = new LockOptions(LockMode.UPGRADE);
+		LockOptions lockOptions = new LockOptions( LockMode.PESSIMISTIC_WRITE );
-		lockOptions.setAliasSpecificLockMode( alias, LockMode.UPGRADE );
+		lockOptions.setAliasSpecificLockMode( alias, LockMode.PESSIMISTIC_WRITE );
 		Map updateTargetColumnsMap = Collections.singletonMap( alias, new String[] { valueColumnName } );
 		return dialect.applyLocksToSql( query, lockOptions, updateTargetColumnsMap );
 	}

core/src/test/java/org/hibernate/id/enhanced/OptimizerUnitTest.java

@@ -1,21 +1,42 @@
-package org.hibernate.test.idgen.enhanced;
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, 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.id.enhanced;
 
 import junit.framework.Test;
+import junit.framework.TestCase;
 import junit.framework.TestSuite;
 
 import org.hibernate.id.IdentifierGeneratorHelper;
 import org.hibernate.id.IntegralDataTypeHolder;
-import org.hibernate.junit.UnitTestCase;
-import org.hibernate.id.enhanced.Optimizer;
-import org.hibernate.id.enhanced.OptimizerFactory;
-import org.hibernate.id.enhanced.AccessCallback;
 
 /**
  * {@inheritDoc}
  *
  * @author Steve Ebersole
  */
-public class OptimizerUnitTest extends UnitTestCase {
+@SuppressWarnings({ "deprecation" })
+public class OptimizerUnitTest extends TestCase {
 	public OptimizerUnitTest(String string) {
 		super( string );
 	}
@@ -99,8 +120,41 @@ public class OptimizerUnitTest extends UnitTestCase {
 		assertEquals( 21, sequence.getCurrentValue() );
 	}
 
+	public void testSubsequentPooledOptimizerUsage() {
+		// test the pooled optimizer in situation where the sequence is already beyond its initial value on init.
+		//		cheat by telling the sequence to start with 1000
+		final SourceMock sequence = new SourceMock( 1000, 3, 5 );
+		//		but tell the optimizer the start-with is 1
+		final Optimizer optimizer = OptimizerFactory.buildOptimizer( OptimizerFactory.POOL, Long.class, 3, 1 );
+
+		assertEquals( 5, sequence.getTimesCalled() );
+		assertEquals( 1000, sequence.getCurrentValue() );
+
+		Long next = (Long) optimizer.generate( sequence );
+		assertEquals( 1000, next.intValue() );
+		assertEquals( (5+1), sequence.getTimesCalled() );
+		assertEquals( (1000+3), sequence.getCurrentValue() );
+
+		next = (Long) optimizer.generate( sequence );
+		assertEquals( 1001, next.intValue() );
+		assertEquals( (5+1), sequence.getTimesCalled() );
+		assertEquals( (1000+3), sequence.getCurrentValue() );
+
+		next = (Long) optimizer.generate( sequence );
+		assertEquals( 1002, next.intValue() );
+		assertEquals( (5+1), sequence.getTimesCalled() );
+		assertEquals( (1000+3), sequence.getCurrentValue() );
+
+		// force a "clock over"
+		next = (Long) optimizer.generate( sequence );
+		assertEquals( 1003, next.intValue() );
+		assertEquals( (5+2), sequence.getTimesCalled() );
+		assertEquals( (1000+6), sequence.getCurrentValue() );
+	}
+
 	private static class SourceMock implements AccessCallback {
 		private IdentifierGeneratorHelper.BasicHolder value = new IdentifierGeneratorHelper.BasicHolder( Long.class );
+		private long initialValue;
 		private int increment;
 		private int timesCalled = 0;
 
@@ -109,21 +163,46 @@ public class OptimizerUnitTest extends UnitTestCase {
 		}
 
 		public SourceMock(long initialValue, int increment) {
+			this( initialValue, increment, 0 );
+		}
+
+		public SourceMock(long initialValue, int increment, int timesCalled) {
 			this.increment = increment;
-			this.value.initialize( initialValue - increment );
+			this.timesCalled = timesCalled;
+			if ( timesCalled != 0 ) {
+				this.value.initialize( initialValue );
+				this.initialValue = 1;
+			}
+			else {
+				this.initialValue = initialValue;
+			}
 		}
 
 		public IntegralDataTypeHolder getNextValue() {
-			timesCalled++;
+			try {
+				if ( timesCalled == 0 ) {
+					initValue();
+					return value.copy();
+				}
+				else {
 					return value.add( increment ).copy();
 				}
+			}
+			finally {
+				timesCalled++;
+			}
+		}
+
+		private void initValue() {
+			this.value.initialize( initialValue );
+		}
 
 		public int getTimesCalled() {
 			return timesCalled;
 		}
 
 		public long getCurrentValue() {
-			return value.getActualLongValue();
+			return value == null ? -1 : value.getActualLongValue();
 		}
 	}
 

core/src/test/java/org/hibernate/id/enhanced/SequenceStyleConfigUnitTest.java

@@ -1,16 +1,12 @@
-package org.hibernate.test.idgen.enhanced;
+package org.hibernate.id.enhanced;
 
 import java.util.Properties;
 
 import junit.framework.Test;
+import junit.framework.TestCase;
 import junit.framework.TestSuite;
 
-import org.hibernate.junit.UnitTestCase;
 import org.hibernate.dialect.Dialect;
-import org.hibernate.id.enhanced.SequenceStyleGenerator;
-import org.hibernate.id.enhanced.SequenceStructure;
-import org.hibernate.id.enhanced.OptimizerFactory;
-import org.hibernate.id.enhanced.TableStructure;
 import org.hibernate.id.PersistentIdentifierGenerator;
 import org.hibernate.Hibernate;
 import org.hibernate.MappingException;
@@ -23,7 +19,8 @@ import org.hibernate.cfg.NamingStrategy;
  *
  * @author Steve Ebersole
  */
-public class SequenceStyleConfigUnitTest extends UnitTestCase {
+@SuppressWarnings({ "deprecation" })
+public class SequenceStyleConfigUnitTest extends TestCase {
 	public SequenceStyleConfigUnitTest(String string) {
 		super( string );
 	}
@@ -32,6 +29,13 @@ public class SequenceStyleConfigUnitTest extends UnitTestCase {
 		return new TestSuite( SequenceStyleConfigUnitTest.class );
 	}
 
+	private void assertClassAssignability(Class expected, Class actual) {
+		if ( ! expected.isAssignableFrom( actual ) ) {
+			fail( "Actual type [" + actual.getName() + "] is not assignable to expected type [" + expected.getName() + "]" );
+		}
+	}
+
+
 	/**
 	 * Test all params defaulted with a dialect supporting sequences
 	 */