Skip to content

Commit

Permalink
Re-use "executedAtLeastOnce" across connections per Brett's suggestion
Browse files Browse the repository at this point in the history
  • Loading branch information
@TobiasSQL
TobiasSQL committed May 31, 2017
1 parent 4bf4c0c commit 38da794
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 124 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/microsoft/sqlserver/jdbc/ParsedSQLMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
119 changes: 40 additions & 79 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -244,30 +212,22 @@ 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. */
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
* false: Reference could not be added, statement has been discarded or does not have a handle associated with it.
* true: Reference was successfully added.
*/
boolean tryAddReference() {
if (!hasHandle() || isDiscarded() || isExplicitlyDiscarded())
if (isDiscarded() || isExplicitlyDiscarded())
return false;
else {
int refCount = handleRefCount.incrementAndGet();
Expand All @@ -285,31 +245,32 @@ void removeReference() {
static final private int PARSED_SQL_CACHE_SIZE = 100;

/** Cache of parsed SQL meta data */
static private ConcurrentLinkedHashMap<Sha1HashKey, ParsedSQLMetadata> parsedSQLCache;
static private ConcurrentLinkedHashMap<Sha1HashKey, ParsedSQLCacheItem> parsedSQLCache;

static {
parsedSQLCache = new Builder<Sha1HashKey, ParsedSQLMetadata>()
parsedSQLCache = new Builder<Sha1HashKey, ParsedSQLCacheItem>()
.maximumWeightedCapacity(PARSED_SQL_CACHE_SIZE)

This comment has been minimized.

Sign in to view

Copy link
@brettwooldridge

brettwooldridge Jun 3, 2017 •
edited
Loading

Contributor

@TobiasSQL I'm thinking that because the isExecutedAtLeastOnce is now based, at least in part, on whether the parsedSQLCache indicates prior execution, maybe the "weighed capacity" (aka "size") of the parsedSQLCache should be the max of PARSED_SQL_CACHE_SIZE and serverPreparedStatementDiscardThreshold.

The only catch is that the parsedSQLCache is initialized statically, when the class is loaded, while the handle "cache" is per-connection.

One solution would be to call:

parsedSQLCache.setCapacity(Math.max(parsedSQLCache.getCapacity(), serverPreparedStatementDiscardThreshold);

wherever serverPreparedStatementDiscardThreshold is modified. The added benefit is that provides an indirect mechanism whereby users can adjust the parsedSQLCache size (upward), similar to MySQL's prepStmtCacheSize (which controls their client-side parse cache as well, see cachePrepStmts).

What do you think?
.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;
Expand Down Expand Up @@ -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();
}


Expand Down Expand Up @@ -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. */
Expand All @@ -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.
}
Expand Down
51 changes: 22 additions & 29 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerPreparedStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
}
}
Expand All @@ -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)) {
Expand All @@ -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)
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 38da794

Please sign in to comment.