Greatly reduce mallocs in SQLString.canonicalizeString()

partially addresses #735

commons-db/src/main/java/com/palantir/nexus/db/sql/SQLString.java

@@ -42,6 +42,7 @@
 
 public class SQLString extends BasicSQLString {
  private static final Pattern ALL_WORD_CHARS_REGEX = Pattern.compile("^[a-zA-Z_0-9\\.\\-]*$"); //$NON-NLS-1$
+ private static final String UNREGISTERED_SQL_COMMENT = "/* UnregisteredSQLString */";
 
  /**
  * Callers changing the value of cachedUnregistered and
@@ -200,7 +201,7 @@ public static boolean isValidKey(final String key) {
  */
  static FinalSQLString getUnregisteredQuery(String sql) {
  assert !isValidKey(sql) : "Unregistered Queries should not look like keys"; //$NON-NLS-1$
- FinalSQLString cached = cachedUnregistered.get(StringUtils.deleteWhitespace(canonicalizeString(sql)));
+ FinalSQLString cached = cachedUnregistered.get(canonicalizeString(sql));
  if(null != cached) {
  callbackOnUse.noteUse((SQLString) cached.delegate);
  return cached;
@@ -251,22 +252,49 @@ private static String makeCommentString(String keyString, DBType dbType) {
  /**
  * Cleans up whitespace, any trailing semicolon, and prefixed comments that a string is
  * unregistered, in order to come up with a canonical representation of this sql string.
- * @param s
- * @return
  */
- public static String canonicalizeString(String s) {
- String trim = s.trim();
- if(!trim.isEmpty() && trim.charAt(trim.length() - 1) == ';') {
- trim = trim.substring(0, trim.length() - 1);
+ public static String canonicalizeString(String sql) {
+ return canonicalizeString(sql, false);
+ }
+
+ static String canonicalizeStringAndRemoveWhitespaceEntirely(String sql) {
+ return canonicalizeString(sql, true);
+ }
+
+ private static String canonicalizeString(String original, boolean removeAllWhitespaceEntirely) {
+ StringBuilder cleanedString = new StringBuilder(original);
+ int originalIdx = 0;
+ int cleanedIdx = 0;
+ int firstUnregisteredIdx = cleanedString.indexOf(UNREGISTERED_SQL_COMMENT);
+
+ while (originalIdx < original.length()) {
+ char originalChar = original.charAt(originalIdx);
+ if (originalIdx == firstUnregisteredIdx) {
+ originalIdx += UNREGISTERED_SQL_COMMENT.length();
+ firstUnregisteredIdx = original.indexOf(UNREGISTERED_SQL_COMMENT, originalIdx);
+ } else if (originalChar == ' ' && (cleanedIdx == 0 || cleanedString.charAt(cleanedIdx - 1) == ' ')
+ || (originalChar == ' ' && removeAllWhitespaceEntirely)) {
+ ++originalIdx;
+ } else {
+ cleanedString.setCharAt(cleanedIdx, originalChar);
+ ++cleanedIdx;
+ ++originalIdx;
+ }
  }
- String prefix = "/* UnregisteredSQLString */"; //$NON-NLS-1$
- if (trim.startsWith(prefix)) {
- trim = trim.substring(prefix.length()).trim();
+
+ if (cleanedString.charAt(cleanedIdx - 1) == ' ') {
+ --cleanedIdx;
  }
- String [] sp = trim.split("\\s+"); //$NON-NLS-1$
- return StringUtils.join(sp, " "); //$NON-NLS-1$
- }
 
+ if (cleanedString.charAt(cleanedIdx - 1) == ';') {
+ --cleanedIdx;
+ if (cleanedString.charAt(cleanedIdx - 1) == ' ') {
+ --cleanedIdx;
+ }
+ }
+
+ return cleanedString.substring(0, cleanedIdx);
+ }
 
  static class NullSQLString extends SQLString {
  final String key;

commons-db/src/test/java/com/palantir/nexus/db/sql/SQLStringTest.java

@@ -0,0 +1,60 @@
+/**
+ * Copyright 2017 Palantir Technologies
+ *
+ * Licensed under the BSD-3 License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://opensource.org/licenses/BSD-3-Clause
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.palantir.nexus.db.sql;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.List;
+
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableList;
+
+public class SQLStringTest {
+ @Test
+ public void testCanonicalizeString() {
+ List<String> testQuery = ImmutableList.of(
+ "insert foo into bar ; ",
+ "insert foo into bar;",
+ "insert foo into bar; ",
+ "/* UnregisteredSQLString */ insert foo into bar;",
+ " /* UnregisteredSQLString */insert foo into bar",
+ " /* UnregisteredSQLString */insert foo into bar ");
+ String canonicalQuery = "insert foo into bar";
+
+ testQuery.forEach(sql -> assertEquals(canonicalQuery, SQLString.canonicalizeString(sql)));
+ }
+
+ @Test
+ public void testCanonicalizeBatch() {
+ List<String> testBatch = ImmutableList.of(
+ "/* UnregisteredSQLString */ insert foo into bar; /* UnregisteredSQLString */insert foo into bar;",
+ "insert foo into bar; /* UnregisteredSQLString */ insert foo into bar");
+ String canconicalBatch = "insert foo into bar; insert foo into bar";
+
+ testBatch.forEach(sql -> assertEquals(canconicalBatch, SQLString.canonicalizeString(sql)));
+ }
+
+ @Test
+ public void testCanonicalizeStringNoSpaces() {
+ List<String> testBatch = ImmutableList.of(
+ "/* UnregisteredSQLString */ insert foo into bar; /* UnregisteredSQLString */insert foo into bar;",
+ "insert foo into bar; /* UnregisteredSQLString */ insert foo into bar");
+ String canconicalBatch = "insertfoointobar;insertfoointobar";
+
+ testBatch.forEach(sql -> assertEquals(canconicalBatch, SQLString.canonicalizeStringAndRemoveWhitespaceEntirely(sql)));
+ }
+}

docs/source/release_notes/release-notes.rst

@@ -42,8 +42,9 @@ develop
  * - Type
  - Change
 
- * -
- -
+ * - |improved|
+ - Improved heap usage during heavy DBKVS querying
+ (Pull Request <https://github.com/palantir/atlasdb/pull/1560>`__)
 
 .. <<<<------------------------------------------------------------------------------------------------------------->>>>