Check the DBM comment region in place, dropping the duplicate-comment substring#11737
Open
dougqh wants to merge 6 commits into
Open
Check the DBM comment region in place, dropping the duplicate-comment substring#11737dougqh wants to merge 6 commits into
dougqh wants to merge 6 commits into
Conversation
dougqh
added
tag: performance
Contributor
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Contributor
Author
|
Benchmark results (zulu-17, JDK 17.0.7,
The |
dougqh
force-pushed
the
dougqh/dbcommenter-scan-overload
branch
from
bd0983f to
24d8ea0
Compare
dougqh
added a commit
that referenced
this pull request
Jun 29, 2026
…ld the view Per review on #11737: move SubSequence.of to the call sites (SQLCommenter builds the comment-region view; the String overload wraps the whole content) and make SharedDBCommenter.containsTraceComment take a SubSequence instead of (String, from, to). Behavior unchanged (SubSequence.contains -> Strings.regionContains); test + dup-comment-benchmark Javadoc updated to the new shape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dougqh
requested review from
jordan-wong and
mhlidd
and removed request for
a team
…tent substring SQLCommenter.hasDDComment materialized sql.substring(commentStart, commentEnd) on every duplicate-comment check just to feed containsTraceComment. Add a SharedDBCommenter.containsTraceComment(sql, from, to) range overload that scans the comment body in place, and a Strings.regionContains primitive it (and the String delegate) build on -- no per-call substring. The String overload now delegates to the range form, so Mongo behavior is unchanged. regionContains is the allocation-free, copy-free primitive; the natural-reading SubSequence.contains layer can delegate to it later. Boundary semantics unit- tested on Strings.regionContains; DB needle behavior on the overloads; existing SQLCommenter/SharedDBCommenter suites unchanged and green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…diom) Add SubSequence.contains (delegating to the Strings.regionContains primitive) and rewrite containsTraceComment(sql, from, to) as SubSequence.of(sql, from, to).contains(...) -- a 1-to-1 substitution for what you'd idiomatically write on a substring, with no copy. EA elides the view in the transient consumption; even if it doesn't, the string copy is still avoided. Couples this branch to the SubSequence base (#10640); regionContains stays as the allocation-free primitive the view delegates to. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Isolates the duplicate-comment guard (dbType=null skips the first-word scan; already-DD-commented SQL makes inject return early). The extractCommentContent substring allocated 140 B/op; the in-place range/view scan is EA-elided (~0). @threads(8), @fork(2), -prof gc; numbers in the class Javadoc. Throughput is flat-to-slightly-up but within @fork(2) noise -- this path is scan-CPU-dominated, so the win is the allocation, not throughput. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dougqh
force-pushed
the
dougqh/dbcommenter-scan-overload
branch
from
71f3df8 to
5904552
Compare
…scan-overload # Conflicts: # internal-api/src/main/java/datadog/trace/util/SubSequence.java # internal-api/src/test/java/datadog/trace/util/SubSequenceTest.java
This comment has been minimized.
This comment has been minimized.
na-ji
approved these changes
Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Does This Do
Replaces the per-call
sql.substringused bySQLCommenter.hasDDComment.In place of a sub-string, the code now uses a
SubSequencewhich has lower overheadMotivation
SubSequenceavoids the allocation and copying of a regular sub-string.And frequently,
SubSequenceis completely optimized away by escape analysis.For this code path, the net effect is zero allocation and a modest ~12% improvement in throughput.
Additional Notes
Measured —
SQLCommenterDuplicateCommentBenchmark(JDK 17,@Threads(8),@Fork(5),-prof gc)gc.alloc.rate.normThe allocation delta (140 → 0 B/op) is the win and is exact/fork-stable. This path is scan-CPU-dominated, so the ~12% throughput is a modest bonus — the real, compounding win is the per-inject allocation removed across comment-bearing queries.
🤖 Generated with Claude Code