diff --git a/dd-java-agent/agent-bootstrap/src/jmh/java/datadog/trace/bootstrap/instrumentation/dbm/SQLCommenterBenchmark.java b/dd-java-agent/agent-bootstrap/src/jmh/java/datadog/trace/bootstrap/instrumentation/dbm/SQLCommenterBenchmark.java new file mode 100644 index 00000000000..5320f3d0a0f --- /dev/null +++ b/dd-java-agent/agent-bootstrap/src/jmh/java/datadog/trace/bootstrap/instrumentation/dbm/SQLCommenterBenchmark.java @@ -0,0 +1,223 @@ +package datadog.trace.bootstrap.instrumentation.dbm; + +import static java.util.concurrent.TimeUnit.NANOSECONDS; +import static java.util.concurrent.TimeUnit.SECONDS; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Benchmarks the trace comment detection and first-word extraction optimizations in SQLCommenter. + * + *
Compares: + * + *
Run with: + * + *
+ * ./gradlew :dd-java-agent:agent-bootstrap:jmhJar + * java -jar dd-java-agent/agent-bootstrap/build/libs/agent-bootstrap-*-jmh.jar SQLCommenterBenchmark + *+ */ +@State(Scope.Benchmark) +@Warmup(iterations = 3, time = 5, timeUnit = SECONDS) +@Measurement(iterations = 5, time = 5, timeUnit = SECONDS) +@BenchmarkMode(Mode.Throughput) +@OutputTimeUnit(NANOSECONDS) +@Fork(value = 1) +public class SQLCommenterBenchmark { + + // Realistic SQL with a prepended DBM comment (the common case for hasDDComment check) + private static final String SQL_WITH_COMMENT = + "/*ddps='myservice',dddbs='mydb',ddh='db-host.example.com',dddb='production_db'," + + "dde='prod',ddpv='1.2.3'*/ SELECT u.id, u.name, u.email FROM users u " + + "WHERE u.active = ? AND u.created_at > ? ORDER BY u.name"; + + // SQL without any comment (the common case for normal queries) + private static final String SQL_NO_COMMENT = + "SELECT u.id, u.name, u.email FROM users u " + + "WHERE u.active = ? AND u.created_at > ? ORDER BY u.name"; + + // SQL with appended comment (MySQL/CALL style) + private static final String SQL_APPENDED_COMMENT = + "CALL get_user_data(?, ?) /*ddps='myservice',dddbs='mydb',ddh='db-host.example.com'," + + "dddb='production_db',dde='prod',ddpv='1.2.3'*/"; + + // Short SQL for first-word extraction benchmarks + private static final String SQL_SELECT = "SELECT * FROM users WHERE id = ?"; + private static final String SQL_CALL = "CALL get_user_data(?, ?)"; + private static final String SQL_BRACE = "{ call get_user_data(?, ?) }"; + + @Param({"prepended", "appended", "none"}) + String commentStyle; + + String sql; + boolean appendComment; + + // Pre-computed indices for the optimized range-based check + int commentStartIdx; + int commentEndIdx; + String commentContent; // Pre-extracted for baseline comparison + + @Setup + public void setup() { + switch (commentStyle) { + case "prepended": + sql = SQL_WITH_COMMENT; + appendComment = false; + break; + case "appended": + sql = SQL_APPENDED_COMMENT; + appendComment = true; + break; + case "none": + default: + sql = SQL_NO_COMMENT; + appendComment = false; + break; + } + + // Pre-compute the comment bounds for the range-based benchmark + if (appendComment) { + commentStartIdx = sql.lastIndexOf("/*"); + commentEndIdx = sql.lastIndexOf("*/"); + } else { + commentStartIdx = sql.indexOf("/*"); + commentEndIdx = sql.indexOf("*/"); + } + + // Pre-extract the comment content for the baseline benchmark + if (commentStartIdx != -1 && commentEndIdx != -1 && commentEndIdx > commentStartIdx) { + commentContent = sql.substring(commentStartIdx + 2, commentEndIdx); + } else { + commentContent = ""; + } + } + + // --- containsTraceComment benchmarks --- + + /** + * Baseline: extract substring then check with String.contains. This is what the old code did via + * extractCommentContent() + containsTraceComment(String). + */ + @Benchmark + @Threads(1) + public boolean containsTraceComment_baseline_substring_1T() { + if (commentStartIdx == -1 || commentEndIdx == -1 || commentEndIdx <= commentStartIdx) { + return false; + } + // Allocates a substring — the old extractCommentContent() behavior + String extracted = sql.substring(commentStartIdx + 2, commentEndIdx); + return SharedDBCommenter.containsTraceComment(extracted); + } + + /** + * Optimized: range-based check with regionMatches, zero allocation. This is what the new code + * does via containsTraceComment(String, int, int). + */ + @Benchmark + @Threads(1) + public boolean containsTraceComment_optimized_range_1T() { + if (commentStartIdx == -1 || commentEndIdx == -1 || commentEndIdx <= commentStartIdx) { + return false; + } + return SharedDBCommenter.containsTraceComment(sql, commentStartIdx + 2, commentEndIdx); + } + + /** Multi-threaded baseline — exposes GC pressure from substring allocation under contention. */ + @Benchmark + @Threads(8) + public boolean containsTraceComment_baseline_substring_8T() { + if (commentStartIdx == -1 || commentEndIdx == -1 || commentEndIdx <= commentStartIdx) { + return false; + } + String extracted = sql.substring(commentStartIdx + 2, commentEndIdx); + return SharedDBCommenter.containsTraceComment(extracted); + } + + /** Multi-threaded optimized — no allocation, no GC pressure. */ + @Benchmark + @Threads(8) + public boolean containsTraceComment_optimized_range_8T() { + if (commentStartIdx == -1 || commentEndIdx == -1 || commentEndIdx <= commentStartIdx) { + return false; + } + return SharedDBCommenter.containsTraceComment(sql, commentStartIdx + 2, commentEndIdx); + } + + // --- firstWord benchmarks --- + + /** + * Baseline: allocate substring via getFirstWord, then call startsWith. This is what the old + * inject() code did. + */ + @Benchmark + @Threads(1) + public boolean firstWord_baseline_substring_1T() { + String firstWord = getFirstWord(sql); + return firstWord.startsWith("{"); + } + + /** Optimized: regionMatches-based check, zero allocation. */ + @Benchmark + @Threads(1) + public boolean firstWord_optimized_regionMatches_1T() { + return firstWordStartsWith(sql, "{"); + } + + /** Multi-threaded baseline — substring allocation under contention. */ + @Benchmark + @Threads(8) + public boolean firstWord_baseline_substring_8T() { + String firstWord = getFirstWord(sql); + return firstWord.startsWith("{"); + } + + /** Multi-threaded optimized — zero allocation. */ + @Benchmark + @Threads(8) + public boolean firstWord_optimized_regionMatches_8T() { + return firstWordStartsWith(sql, "{"); + } + + // --- Inlined helper methods (mirror the implementations for fair comparison) --- + + /** Original getFirstWord — allocates a substring. */ + private static String getFirstWord(String sql) { + int beginIndex = 0; + while (beginIndex < sql.length() && Character.isWhitespace(sql.charAt(beginIndex))) { + beginIndex++; + } + int endIndex = beginIndex; + while (endIndex < sql.length() && !Character.isWhitespace(sql.charAt(endIndex))) { + endIndex++; + } + return sql.substring(beginIndex, endIndex); + } + + /** Optimized firstWordStartsWith — zero allocation via regionMatches. */ + private static boolean firstWordStartsWith(String sql, String prefix) { + int beginIndex = 0; + int len = sql.length(); + while (beginIndex < len && Character.isWhitespace(sql.charAt(beginIndex))) { + beginIndex++; + } + return sql.regionMatches(beginIndex, prefix, 0, prefix.length()); + } +} diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/dbm/SharedDBCommenter.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/dbm/SharedDBCommenter.java index 83604a017bd..72ac2f16daf 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/dbm/SharedDBCommenter.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/dbm/SharedDBCommenter.java @@ -32,17 +32,56 @@ public class SharedDBCommenter { private static final String TRACEPARENT = encode("traceparent"); private static final String DD_SERVICE_HASH = encode("ddsh"); + // Pre-computed marker strings for trace comment detection + private static final String PARENT_SERVICE_EQ = PARENT_SERVICE + "="; + private static final String DATABASE_SERVICE_EQ = DATABASE_SERVICE + "="; + private static final String DD_HOSTNAME_EQ = DD_HOSTNAME + "="; + private static final String DD_DB_NAME_EQ = DD_DB_NAME + "="; + private static final String DD_PEER_SERVICE_EQ = DD_PEER_SERVICE + "="; + private static final String DD_ENV_EQ = DD_ENV + "="; + private static final String DD_VERSION_EQ = DD_VERSION + "="; + private static final String TRACEPARENT_EQ = TRACEPARENT + "="; + private static final String DD_SERVICE_HASH_EQ = DD_SERVICE_HASH + "="; + // Used by SQLCommenter and MongoCommentInjector to avoid duplicate comment injection public static boolean containsTraceComment(String commentContent) { - return commentContent.contains(PARENT_SERVICE + "=") - || commentContent.contains(DATABASE_SERVICE + "=") - || commentContent.contains(DD_HOSTNAME + "=") - || commentContent.contains(DD_DB_NAME + "=") - || commentContent.contains(DD_PEER_SERVICE + "=") - || commentContent.contains(DD_ENV + "=") - || commentContent.contains(DD_VERSION + "=") - || commentContent.contains(TRACEPARENT + "=") - || commentContent.contains(DD_SERVICE_HASH + "="); + return commentContent.contains(PARENT_SERVICE_EQ) + || commentContent.contains(DATABASE_SERVICE_EQ) + || commentContent.contains(DD_HOSTNAME_EQ) + || commentContent.contains(DD_DB_NAME_EQ) + || commentContent.contains(DD_PEER_SERVICE_EQ) + || commentContent.contains(DD_ENV_EQ) + || commentContent.contains(DD_VERSION_EQ) + || commentContent.contains(TRACEPARENT_EQ) + || commentContent.contains(DD_SERVICE_HASH_EQ); + } + + /** + * Checks for trace comment markers within a range of the given string, without allocating a + * substring. Searches within [fromIndex, toIndex) of the source string. + */ + public static boolean containsTraceComment(String sql, int fromIndex, int toIndex) { + return containsInRange(sql, PARENT_SERVICE_EQ, fromIndex, toIndex) + || containsInRange(sql, DATABASE_SERVICE_EQ, fromIndex, toIndex) + || containsInRange(sql, DD_HOSTNAME_EQ, fromIndex, toIndex) + || containsInRange(sql, DD_DB_NAME_EQ, fromIndex, toIndex) + || containsInRange(sql, DD_PEER_SERVICE_EQ, fromIndex, toIndex) + || containsInRange(sql, DD_ENV_EQ, fromIndex, toIndex) + || containsInRange(sql, DD_VERSION_EQ, fromIndex, toIndex) + || containsInRange(sql, TRACEPARENT_EQ, fromIndex, toIndex) + || containsInRange(sql, DD_SERVICE_HASH_EQ, fromIndex, toIndex); + } + + /** Checks if {@code target} appears within the range [fromIndex, toIndex) of {@code source}. */ + private static boolean containsInRange(String source, String target, int fromIndex, int toIndex) { + int targetLen = target.length(); + int limit = toIndex - targetLen; + for (int i = fromIndex; i <= limit; i++) { + if (source.regionMatches(i, target, 0, targetLen)) { + return true; + } + } + return false; } // Build database comment content without comment delimiters such as /* */ diff --git a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java index 3171550aba3..e469a213360 100644 --- a/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java +++ b/dd-java-agent/instrumentation/jdbc/src/main/java/datadog/trace/instrumentation/jdbc/SQLCommenter.java @@ -15,6 +15,11 @@ public class SQLCommenter { private static final int BUFFER_EXTRA = 4; private static final int SQL_COMMENT_OVERHEAD = SPACE_CHARS + COMMENT_DELIMITERS + BUFFER_EXTRA; + /** + * Returns the first non-whitespace word in the SQL string. The result is a substring allocation, + * so callers that only need to check prefix/equality should use {@link #firstWordStartsWith} or + * {@link #firstWordEqualsIgnoreCase} instead. + */ protected static String getFirstWord(String sql) { int beginIndex = 0; while (beginIndex < sql.length() && Character.isWhitespace(sql.charAt(beginIndex))) { @@ -27,6 +32,38 @@ protected static String getFirstWord(String sql) { return sql.substring(beginIndex, endIndex); } + /** + * Checks if the first non-whitespace word in sql starts with the given prefix, without allocating + * a substring. + */ + private static boolean firstWordStartsWith(String sql, String prefix) { + int beginIndex = 0; + int len = sql.length(); + while (beginIndex < len && Character.isWhitespace(sql.charAt(beginIndex))) { + beginIndex++; + } + return sql.regionMatches(beginIndex, prefix, 0, prefix.length()); + } + + /** + * Checks if the first non-whitespace word in sql equals the given target (case-insensitive), + * without allocating a substring. + */ + private static boolean firstWordEqualsIgnoreCase(String sql, String target) { + int beginIndex = 0; + int len = sql.length(); + while (beginIndex < len && Character.isWhitespace(sql.charAt(beginIndex))) { + beginIndex++; + } + int endIndex = beginIndex; + while (endIndex < len && !Character.isWhitespace(sql.charAt(endIndex))) { + endIndex++; + } + int wordLen = endIndex - beginIndex; + return wordLen == target.length() + && sql.regionMatches(true, beginIndex, target, 0, target.length()); + } + public static String inject( String sql, String dbService, @@ -40,25 +77,25 @@ public static String inject( } boolean appendComment = preferAppend; if (dbType != null) { - final String firstWord = getFirstWord(sql); + boolean startsWithBrace = firstWordStartsWith(sql, "{"); // The Postgres JDBC parser doesn't allow SQL comments anywhere in a JDBC // callable statements // https://github.com/pgjdbc/pgjdbc/blob/master/pgjdbc/src/main/java/org/postgresql/core/Parser.java#L1038 // TODO: Could we inject the comment after the JDBC has been converted to // standard SQL? - if (firstWord.startsWith("{") && dbType.startsWith("postgres")) { + if (startsWithBrace && dbType.startsWith("postgres")) { return sql; } // Append the comment for mysql JDBC callable statements - if (firstWord.startsWith("{") && "mysql".equals(dbType)) { + if (startsWithBrace && "mysql".equals(dbType)) { appendComment = true; } // Both Postgres and MySQL are unhappy with anything before CALL in a stored // procedure invocation, but they seem ok with it after so we force append mode - if (firstWord.equalsIgnoreCase("call")) { + if (firstWordEqualsIgnoreCase(sql, "call")) { appendComment = true; } @@ -112,11 +149,6 @@ private static boolean hasDDComment(String sql, boolean appendComment) { return false; } - String commentContent = extractCommentContent(sql, appendComment); - return SharedDBCommenter.containsTraceComment(commentContent); - } - - private static String extractCommentContent(String sql, boolean appendComment) { int startIdx; int endIdx; if (appendComment) { @@ -127,9 +159,11 @@ private static String extractCommentContent(String sql, boolean appendComment) { endIdx = sql.indexOf(CLOSE_COMMENT); } if (startIdx != -1 && endIdx != -1 && endIdx > startIdx) { - return sql.substring(startIdx + OPEN_COMMENT_LEN, endIdx); + // Check for trace comment markers directly in the SQL without allocating a substring. + // We search within the bounds [startIdx + OPEN_COMMENT_LEN, endIdx) of the original string. + return SharedDBCommenter.containsTraceComment(sql, startIdx + OPEN_COMMENT_LEN, endIdx); } - return ""; + return false; } /**