Skip to content

perf(spanner-jdbc): cache positional to named param conversion #14034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
olavloite wants to merge 1 commit into
base: main
Choose a base branch
from
+80 −2
Open

perf(spanner-jdbc): cache positional to named param conversion #14034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions ...ud-spanner/src/main/java/com/google/cloud/spanner/connection/AbstractStatementParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,13 @@ private static boolean isRecordStatementCacheStats() {
* Cache for parsed statements. This prevents statements that are executed multiple times by the
* application to be parsed over and over again. The default maximum size is 5Mb.
*/
private final Cache<String, ParsedStatement> statementCache;
@Nullable private final Cache<String, ParsedStatement> statementCache;

/**
* Cache for positional parameters info. This prevents statements that are executed multiple times
* using positional parameters from having to be scanned and translated repeatedly.
*/
@Nullable private final Cache<String, ParametersInfo> positionalParametersCache;

AbstractStatementParser(Set<ClientSideStatementImpl> statements) {
this.statements = Collections.unmodifiableSet(statements);
Expand All @@ -502,16 +508,36 @@ private static boolean isRecordStatementCacheStats() {
cacheBuilder.recordStats();
}
this.statementCache = cacheBuilder.build();

CacheBuilder<String, ParametersInfo> positionalCacheBuilder =
CacheBuilder.newBuilder()
.maximumWeight(maxCacheSize * 1024L * 1024L)
Comment thread
olavloite marked this conversation as resolved.
.weigher(
(String key, ParametersInfo value) ->
2 * key.length() + 2 * value.sqlWithNamedParameters.length())
.concurrencyLevel(Runtime.getRuntime().availableProcessors());
if (isRecordStatementCacheStats()) {
positionalCacheBuilder.recordStats();
}
this.positionalParametersCache = positionalCacheBuilder.build();
} else {
this.statementCache = null;
this.positionalParametersCache = null;
}
}

@VisibleForTesting
@Nullable
CacheStats getStatementCacheStats() {
return statementCache == null ? null : statementCache.stats();
}

@VisibleForTesting
@Nullable
CacheStats getPositionalParametersCacheStats() {
return positionalParametersCache == null ? null : positionalParametersCache.stats();
}

@VisibleForTesting
Set<ClientSideStatementImpl> getClientSideStatements() {
return statements;
Expand Down Expand Up @@ -776,8 +802,21 @@ public static class ParametersInfo {
@InternalApi
public ParametersInfo convertPositionalParametersToNamedParameters(char paramChar, String sql) {
Preconditions.checkNotNull(sql);
if (positionalParametersCache == null) {
return internalConvertPositionalParametersToNamedParameters(paramChar, sql);
}
String cacheKey = paramChar == '?' ? sql : paramChar + "\0" + sql;
ParametersInfo info = positionalParametersCache.getIfPresent(cacheKey);
if (info == null) {
info = internalConvertPositionalParametersToNamedParameters(paramChar, sql);
positionalParametersCache.put(cacheKey, info);
}
return info;
}

ParametersInfo internalConvertPositionalParametersToNamedParameters(char paramChar, String sql) {
final String namedParamPrefix = getQueryParameterPrefix();
StringBuilder named = new StringBuilder(sql.length() + countOccurrencesOf(paramChar, sql));
StringBuilder named = new StringBuilder(sql.length() + 32);
int index = 0;
int paramIndex = 1;
while (index < sql.length()) {
Expand Down
39 changes: 39 additions & 0 deletions ...-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
Expand All @@ -29,6 +30,7 @@
import com.google.cloud.spanner.ErrorCode;
import com.google.cloud.spanner.SpannerException;
import com.google.cloud.spanner.Statement;
import com.google.cloud.spanner.connection.AbstractStatementParser.ParametersInfo;
import com.google.cloud.spanner.connection.AbstractStatementParser.ParsedStatement;
import com.google.cloud.spanner.connection.AbstractStatementParser.StatementType;
import com.google.cloud.spanner.connection.ClientSideStatementImpl.CompileException;
Expand Down Expand Up @@ -1864,6 +1866,43 @@ public void testStatementCache_ParameterizedStatement() {
assertEquals(1, stats.hitCount());
}

@Test
public void testPositionalParametersCache() {
CacheStats statsBefore = parser.getPositionalParametersCacheStats();

String sql = "select * from foo where id=? and name=? and value=" + UUID.randomUUID();
ParametersInfo info1 = parser.convertPositionalParametersToNamedParameters('?', sql);
assertEquals(2, info1.numberOfParameters);
if (dialect == Dialect.POSTGRESQL) {
assertTrue(info1.sqlWithNamedParameters.contains("$1"));
assertTrue(info1.sqlWithNamedParameters.contains("$2"));
} else {
assertTrue(info1.sqlWithNamedParameters.contains("@p1"));
assertTrue(info1.sqlWithNamedParameters.contains("@p2"));
}

ParametersInfo info2 = parser.convertPositionalParametersToNamedParameters('?', sql);
assertEquals(info1.numberOfParameters, info2.numberOfParameters);
assertEquals(info1.sqlWithNamedParameters, info2.sqlWithNamedParameters);
assertSame(info1, info2);

// Test with non-'?' parameter character.
String sqlDollar = "select * from foo where id=$ and name=$ and value=" + UUID.randomUUID();
ParametersInfo infoDollar1 =
parser.convertPositionalParametersToNamedParameters('$', sqlDollar);
assertEquals(2, infoDollar1.numberOfParameters);
ParametersInfo infoDollar2 =
parser.convertPositionalParametersToNamedParameters('$', sqlDollar);
assertSame(infoDollar1, infoDollar2);

CacheStats statsAfter = parser.getPositionalParametersCacheStats();
CacheStats stats = statsAfter.minus(statsBefore);

// Two distinct queries had cache misses. Two repeated queries had cache hits.
assertEquals(2, stats.missCount());
assertEquals(2, stats.hitCount());
}

@Test
public void testClientSideStatementWithComment() {
String sql = "-- Null (no timeout)\n" + "SET STATEMENT_TIMEOUT=null";
Expand Down
Loading