From 38da7940f7de2e8ea549b57d6790d40a195465e8 Mon Sep 17 00:00:00 2001 From: tobiast Date: Wed, 31 May 2017 15:38:59 -0700 Subject: [PATCH] Re-use "executedAtLeastOnce" across connections per Brett's suggestion --- .../sqlserver/jdbc/ParsedSQLMetadata.java | 4 +- .../sqlserver/jdbc/SQLServerConnection.java | 119 ++++++------------ .../jdbc/SQLServerPreparedStatement.java | 51 ++++---- .../sqlserver/jdbc/SQLServerStatement.java | 7 +- .../unit/statement/PreparedStatementTest.java | 54 ++++++-- 5 files changed, 111 insertions(+), 124 deletions(-) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/ParsedSQLMetadata.java b/src/main/java/com/microsoft/sqlserver/jdbc/ParsedSQLMetadata.java index f047ff71a..19c34ebec 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/ParsedSQLMetadata.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/ParsedSQLMetadata.java @@ -11,14 +11,14 @@ /** * Used for caching of meta data from parsed SQL text. */ -final class ParsedSQLMetadata { +final class ParsedSQLCacheItem { /** The SQL text AFTER processing. */ String processedSQL; int parameterCount; String procedureName; boolean bReturnValueSyntax; - ParsedSQLMetadata(String processedSQL, int parameterCount, String procedureName, boolean bReturnValueSyntax) { + ParsedSQLCacheItem(String processedSQL, int parameterCount, String procedureName, boolean bReturnValueSyntax) { this.processedSQL = processedSQL; this.parameterCount = parameterCount; this.procedureName = procedureName; diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java index e871db731..7ab9619d5 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java @@ -157,22 +157,16 @@ class PreparedStatementHandle { private int handle = 0; private final AtomicInteger handleRefCount = new AtomicInteger(); private boolean isDirectSql; - private volatile boolean hasExecutedAtLeastOnce; private volatile boolean evictedFromCache; private volatile boolean explicitlyDiscarded; private Sha1HashKey key; - - PreparedStatementHandle(Sha1HashKey key) { - this.key = key; - } PreparedStatementHandle(Sha1HashKey key, int handle, boolean isDirectSql, boolean isEvictedFromCache) { - this(key); - + this.key = key; this.handle = handle; this.isDirectSql = isDirectSql; - this.setHasExecutedAtLeastOnce(true); this.setIsEvictedFromCache(isEvictedFromCache); + handleRefCount.set(1); } /** Has the statement been evicted from the statement handle cache. */ @@ -197,16 +191,6 @@ private boolean isExplicitlyDiscarded() { return explicitlyDiscarded; } - /** Has the statement that this instance is related to ever been executed (with or without handle) */ - boolean hasExecutedAtLeastOnce() { - return hasExecutedAtLeastOnce; - } - - /** Specify whether the statement that this instance is related to ever been executed (with or without handle) */ - void setHasExecutedAtLeastOnce(boolean hasExecutedAtLeastOnce) { - this.hasExecutedAtLeastOnce = hasExecutedAtLeastOnce; - } - /** Get the actual handle. */ int getHandle() { return handle; @@ -217,22 +201,6 @@ Sha1HashKey getKey() { return key; } - /** Specify the handle. - * - * @return - * false: Handle could not be referenced (already references other handle). - * true: Handle was successfully set. - */ - boolean setHandle(int handle, boolean isDirectSql) { - if (handleRefCount.compareAndSet(0, 1)) { - this.handle = handle; - this.isDirectSql = isDirectSql; - return true; - } - else - return false; - } - boolean isDirectSql() { return isDirectSql; } @@ -244,10 +212,7 @@ boolean isDirectSql() { * true: Handle was successfully put on path for discarding. */ private boolean tryDiscardHandle() { - if(!hasHandle()) - return false; - else - return handleRefCount.compareAndSet(0, -999); + return handleRefCount.compareAndSet(0, -999); } /** Returns whether this statement has been discarded and can no longer be re-used. */ @@ -255,11 +220,6 @@ private boolean isDiscarded() { return 0 > handleRefCount.intValue(); } - /** Returns whether this statement has an actual server handle associated with it. */ - boolean hasHandle() { - return 0 < getHandle(); - } - /** Adds a new reference to this handle, i.e. re-using it. * * @return @@ -267,7 +227,7 @@ boolean hasHandle() { * true: Reference was successfully added. */ boolean tryAddReference() { - if (!hasHandle() || isDiscarded() || isExplicitlyDiscarded()) + if (isDiscarded() || isExplicitlyDiscarded()) return false; else { int refCount = handleRefCount.incrementAndGet(); @@ -285,31 +245,32 @@ void removeReference() { static final private int PARSED_SQL_CACHE_SIZE = 100; /** Cache of parsed SQL meta data */ - static private ConcurrentLinkedHashMap parsedSQLCache; + static private ConcurrentLinkedHashMap parsedSQLCache; static { - parsedSQLCache = new Builder() + parsedSQLCache = new Builder() .maximumWeightedCapacity(PARSED_SQL_CACHE_SIZE) .build(); } - /** Get prepared statement cache entry if exists, if not parse and create a new one */ - static ParsedSQLMetadata getOrCreateCachedParsedSQLMetadata(Sha1HashKey key, String sql) throws SQLServerException { - ParsedSQLMetadata cacheItem = parsedSQLCache.get(key); - if (null == cacheItem) { - JDBCSyntaxTranslator translator = new JDBCSyntaxTranslator(); - - String parsedSql = translator.translate(sql); - String procName = translator.getProcedureName(); // may return null - boolean returnValueSyntax = translator.hasReturnValueSyntax(); - int paramCount = countParams(parsedSql); - - cacheItem = new ParsedSQLMetadata(parsedSql, paramCount, procName, returnValueSyntax); - parsedSQLCache.putIfAbsent(key, cacheItem); - } - - return cacheItem; - } + /** Get prepared statement cache entry if exists, if not parse and create a new one */ + static ParsedSQLCacheItem getCachedParsedSQL(Sha1HashKey key) { + return parsedSQLCache.get(key); + } + + /** Parse and create a information about parsed SQL text */ + static ParsedSQLCacheItem parseAndCacheSQL(Sha1HashKey key, String sql) throws SQLServerException { + JDBCSyntaxTranslator translator = new JDBCSyntaxTranslator(); + + String parsedSql = translator.translate(sql); + String procName = translator.getProcedureName(); // may return null + boolean returnValueSyntax = translator.hasReturnValueSyntax(); + int paramCount = countParams(parsedSql); + + ParsedSQLCacheItem cacheItem = new ParsedSQLCacheItem (parsedSql, paramCount, procName, returnValueSyntax); + parsedSQLCache.putIfAbsent(key, cacheItem); + return cacheItem; + } /** Size of the prepared statement handle cache */ private int statementPoolingCacheSize = 10; @@ -5501,10 +5462,8 @@ final void enqueueUnprepareStatementHandle(PreparedStatementHandle statementHand loggerExternal.finer(this + ": Adding PreparedHandle to queue for un-prepare:" + statementHandle.getHandle()); // Add the new handle to the discarding queue and find out current # enqueued. - if(statementHandle.hasHandle()) { - this.discardedPreparedStatementHandles.add(statementHandle); - this.discardedPreparedStatementHandleCount.incrementAndGet(); - } + this.discardedPreparedStatementHandles.add(statementHandle); + this.discardedPreparedStatementHandleCount.incrementAndGet(); } @@ -5708,27 +5667,29 @@ final void registerCachedParameterMetadata(Sha1HashKey key, SQLServerParameterMe } /** Get or create prepared statement handle cache entry if statement pooling is enabled */ - final PreparedStatementHandle getOrRegisterCachedPreparedStatementHandle(Sha1HashKey key) { + final PreparedStatementHandle getCachedPreparedStatementHandle(Sha1HashKey key) { if(!isStatementPoolingEnabled()) return null; - PreparedStatementHandle cacheItem = preparedStatementHandleCache.get(key); - if (null == cacheItem) { - cacheItem = new PreparedStatementHandle(key); - preparedStatementHandleCache.putIfAbsent(key, cacheItem); - } + return preparedStatementHandleCache.get(key); + } + /** Get or create prepared statement handle cache entry if statement pooling is enabled */ + final PreparedStatementHandle registerCachedPreparedStatementHandle(Sha1HashKey key, int handle, boolean isDirectSql) { + if(!isStatementPoolingEnabled() || null == key) + return null; + + PreparedStatementHandle cacheItem = new PreparedStatementHandle(key, handle, isDirectSql, false); + preparedStatementHandleCache.putIfAbsent(key, cacheItem); return cacheItem; } /** Return prepared statement handle cache entry so it can be un-prepared. */ final void returnCachedPreparedStatementHandle(PreparedStatementHandle handle) { - if(handle.hasHandle()) { - handle.removeReference(); + handle.removeReference(); - if (handle.isEvictedFromCache() && handle.tryDiscardHandle()) - enqueueUnprepareStatementHandle(handle); - } + if (handle.isEvictedFromCache() && handle.tryDiscardHandle()) + enqueueUnprepareStatementHandle(handle); } /** Force eviction of prepared statement handle cache entry. */ @@ -5746,7 +5707,7 @@ public void onEviction(Sha1HashKey key, PreparedStatementHandle handle) { handle.setIsEvictedFromCache(true); // Mark as evicted from cache. // Only discard if not referenced. - if(handle.hasHandle() && handle.tryDiscardHandle()) { + if(handle.tryDiscardHandle()) { enqueueUnprepareStatementHandle(handle); // Do not run discard actions here! Can interfere with executing statement. } diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java index b0755b06a..881448b9b 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java @@ -8,7 +8,8 @@ package com.microsoft.sqlserver.jdbc; -import static com.microsoft.sqlserver.jdbc.SQLServerConnection.getOrCreateCachedParsedSQLMetadata; +import static com.microsoft.sqlserver.jdbc.SQLServerConnection.getCachedParsedSQL; +import static com.microsoft.sqlserver.jdbc.SQLServerConnection.parseAndCacheSQL; import java.io.InputStream; import java.io.Reader; @@ -175,7 +176,13 @@ String getClassNameInternal() { sqlTextCacheKey = new Sha1HashKey(sql); // Parse or fetch SQL metadata from cache. - ParsedSQLMetadata parsedSQL = getOrCreateCachedParsedSQLMetadata(sqlTextCacheKey, sql); + ParsedSQLCacheItem parsedSQL = getCachedParsedSQL(sqlTextCacheKey); + if(null != parsedSQL) { + isExecutedAtLeastOnce = true; + } + else { + parsedSQL = parseAndCacheSQL(sqlTextCacheKey, sql); + } // Retrieve meta data from cache item. procedureName = parsedSQL.procedureName; @@ -568,11 +575,9 @@ boolean onRetValue(TDSReader tdsReader) throws SQLServerException { setPreparedStatementHandle(param.getInt(tdsReader)); - // Check if a cache reference should be updated with the newly created handle, NOT for cursorable handles. - if (null != cachedPreparedStatementHandle && !isCursorable(executeMethod)) { - // Attempt to update the handle, if the update fails remove the reference to the cache item since it references a different handle. - if (!cachedPreparedStatementHandle.setHandle(prepStmtHandle, executedSqlDirectly)) - cachedPreparedStatementHandle = null; // Handle could not be set, treat as not cached. + // Cache the reference to the newly created handle, NOT for cursorable handles. + if (null == cachedPreparedStatementHandle && !isCursorable(executeMethod)) { + cachedPreparedStatementHandle = connection.registerCachedPreparedStatementHandle(new Sha1HashKey(preparedSQL, preparedTypeDefinitions), prepStmtHandle, executedSqlDirectly); } param.skipValue(tdsReader, true); @@ -902,8 +907,7 @@ private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discard // If current cache item should be discarded make sure it is not used again. if (discardCurrentCacheItem && null != cachedPreparedStatementHandle) { - if(cachedPreparedStatementHandle.hasHandle()) - cachedPreparedStatementHandle.removeReference(); + cachedPreparedStatementHandle.removeReference(); // Make sure the cached handle does not get re-used more. resetPrepStmtHandle(); @@ -923,21 +927,16 @@ private boolean reuseCachedHandle(boolean hasNewTypeDefinitions, boolean discard // Check for new cache reference. if (null == cachedPreparedStatementHandle) { - cachedPreparedStatementHandle = connection.getOrRegisterCachedPreparedStatementHandle(new Sha1HashKey(preparedSQL, preparedTypeDefinitions)); + PreparedStatementHandle cachedHandle = connection.getCachedPreparedStatementHandle(new Sha1HashKey(preparedSQL, preparedTypeDefinitions)); // If handle was found then re-use. - if (null != cachedPreparedStatementHandle) { - - // Because sp_executesql was already called on this SQL-text use - // regular prep/exec pattern. - if (cachedPreparedStatementHandle.hasExecutedAtLeastOnce()) - isExecutedAtLeastOnce = true; - - // If existing handle was found and we can add reference to it, use - // it. - if (cachedPreparedStatementHandle.tryAddReference()) { - setPreparedStatementHandle(cachedPreparedStatementHandle.getHandle()); - return true; + if (null != cachedHandle) { + + // If existing handle was found and we can add reference to it, use it. + if (cachedHandle.tryAddReference()) { + setPreparedStatementHandle(cachedHandle.getHandle()); + cachedPreparedStatementHandle = cachedHandle; + return true; } } } @@ -949,8 +948,7 @@ private boolean doPrepExec(TDSWriter tdsWriter, boolean hasNewTypeDefinitions, boolean hasExistingTypeDefinitions) throws SQLServerException { - boolean hasHandle = hasPreparedStatementHandle(); - boolean needsPrepare = (hasNewTypeDefinitions && hasExistingTypeDefinitions) || !hasHandle; + boolean needsPrepare = (hasNewTypeDefinitions && hasExistingTypeDefinitions) || !hasPreparedStatementHandle(); // Cursors don't use statement pooling. if (isCursorable(executeMethod)) { @@ -969,11 +967,6 @@ private boolean doPrepExec(TDSWriter tdsWriter, ) { buildExecSQLParams(tdsWriter); isExecutedAtLeastOnce = true; - - // Enable re-use if caching is on by moving to sp_prepexec on next call even from separate instance. - if (null != cachedPreparedStatementHandle) { - cachedPreparedStatementHandle.setHasExecutedAtLeastOnce(true); - } } // Second execution, use prepared statements since we seem to be re-using it. else if(needsPrepare) diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java index 6d1f572f9..81718b73e 100644 --- a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java +++ b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java @@ -8,7 +8,8 @@ package com.microsoft.sqlserver.jdbc; -import static com.microsoft.sqlserver.jdbc.SQLServerConnection.getOrCreateCachedParsedSQLMetadata; +import static com.microsoft.sqlserver.jdbc.SQLServerConnection.getCachedParsedSQL; +import static com.microsoft.sqlserver.jdbc.SQLServerConnection.parseAndCacheSQL; import java.sql.BatchUpdateException; import java.sql.ResultSet; @@ -769,7 +770,9 @@ private String ensureSQLSyntax(String sql) throws SQLServerException { Sha1HashKey cacheKey = new Sha1HashKey(sql); // Check for cached SQL metadata. - ParsedSQLMetadata cacheItem = getOrCreateCachedParsedSQLMetadata(cacheKey, sql); + ParsedSQLCacheItem cacheItem = getCachedParsedSQL(cacheKey); + if (null == cacheItem) + cacheItem = parseAndCacheSQL(cacheKey, sql); // Retrieve from cache item. procedureName = cacheItem.procedureName; diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java index 6cb0eb812..c274e3611 100644 --- a/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java +++ b/src/test/java/com/microsoft/sqlserver/jdbc/unit/statement/PreparedStatementTest.java @@ -18,6 +18,7 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.util.Random; import java.util.UUID; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -31,6 +32,7 @@ import com.microsoft.sqlserver.jdbc.SQLServerDataSource; import com.microsoft.sqlserver.jdbc.SQLServerPreparedStatement; import com.microsoft.sqlserver.testframework.AbstractTest; +import com.microsoft.sqlserver.testframework.util.RandomUtil; @RunWith(JUnitPlatform.class) public class PreparedStatementTest extends AbstractTest { @@ -81,17 +83,6 @@ public void testBatchedUnprepare() throws SQLException { int iterations = 25; - // Verify no prepares for 1 time only uses. - for(int i = 0; i < iterations; ++i) { - try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(query)) { - pstmt.execute(); - } - assertSame(0, con.getDiscardedServerPreparedStatementCount()); - } - - // Verify total cache use. - assertSame(iterations, executeSQLReturnFirstInt(con, verifyTotalCacheUsesQuery)); - query = String.format("/*unpreparetest_%s, sp_executesql->sp_prepexec->sp_execute- batched sp_unprepare*/SELECT * FROM sys.tables;", lookupUniqueifier); int prevDiscardActionCount = 0; @@ -101,7 +92,7 @@ public void testBatchedUnprepare() throws SQLException { // Verify current queue depth is expected. assertSame(prevDiscardActionCount, con.getDiscardedServerPreparedStatementCount()); - try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(query)) { + try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement)con.prepareStatement(String.format("%s--%s", query, i))) { pstmt.execute(); // sp_executesql pstmt.execute(); // sp_prepexec @@ -140,6 +131,45 @@ public void testBatchedUnprepare() throws SQLException { */ @Test public void testStatementPooling() throws SQLException { + // Test % handle re-use + try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { + String query = String.format("/*statementpoolingtest_re-use_%s*/SELECT TOP(1) * FROM sys.tables;", UUID.randomUUID().toString()); + + con.setStatementPoolingCacheSize(10); + + boolean[] prepOnFirstCalls = {false, true}; + + for(boolean prepOnFirstCall : prepOnFirstCalls) { + + con.setEnablePrepareOnFirstPreparedStatementCall(prepOnFirstCall); + + int[] queryCounts = {10, 20, 30, 40}; + for(int queryCount : queryCounts) { + String[] queries = new String[queryCount]; + for(int i = 0; i < queries.length; ++i) { + queries[i] = String.format("%s--%s--%s--%s", query, i, queryCount, prepOnFirstCall); + } + + int testsWithHandleReuse = 0; + final int testCount = 500; + for(int i = 0; i < testCount; ++i) { + Random random = new Random(); + int queryNumber = random.nextInt(queries.length); + try (SQLServerPreparedStatement pstmt = (SQLServerPreparedStatement) con.prepareStatement(queries[queryNumber])) { + pstmt.execute(); + + // Grab handle-reuse before it would be populated if initially created. + if(0 < pstmt.getPreparedStatementHandle()) + testsWithHandleReuse++; + + pstmt.getMoreResults(); // Make sure handle is updated. + } + } + System.out.println(String.format("Prep on first call: %s Query count:%s: %s of %s (%s)", prepOnFirstCall, queryCount, testsWithHandleReuse, testCount, (double)testsWithHandleReuse/(double)testCount)); + } + } + } + try (SQLServerConnection con = (SQLServerConnection)DriverManager.getConnection(connectionString)) { // Test behvaior with statement pooling.