diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/DDSpanContextSetTagBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/DDSpanContextSetTagBenchmark.java new file mode 100644 index 00000000000..2ee2ac079e9 --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/DDSpanContextSetTagBenchmark.java @@ -0,0 +1,81 @@ +package datadog.trace.core; + +import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.common.writer.Writer; +import java.util.List; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +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; + +/** + * Benchmark for {@code DDSpanContext.setTag} on a primitive value -- the path the caller + * collapse touched. Before: {@code precheckIntercept} (an {@code isOptimized} call + a {@code + * needsIntercept} resolve) and then, when intercepted, a second resolve inside {@code + * interceptTag}. After: one {@code handlerId} resolve, then {@code handleIntercept(handlerId)} + * directly. + * + *

Two paths: intercepted (e.g. {@code http.status_code} -- exercises the + * double->single resolve), and notIntercepted (an app metric -- exercises the dropped + * {@code isOptimized} call; stores the primitive without boxing). Per-thread span so the (mutating) + * {@code setTag} has no cross-thread contention under {@code @Threads(8)}. + * + *

Run before (pre-collapse DDSpanContext) vs after by toggling DDSpanContext.java. + * + *

+ *   ./gradlew :dd-trace-core:jmh   # add -prof gc
+ * 
+ */ +@Fork(2) +@Warmup(iterations = 2) +@Measurement(iterations = 5) +@Threads(8) +public class DDSpanContextSetTagBenchmark { + + private static final CoreTracer TRACER = + CoreTracer.builder().writer(new NoopWriter()).strictTraceWrites(false).build(); + + @State(Scope.Thread) + public static class SpanState { + DDSpan span; + + @Setup + public void setup() { + this.span = (DDSpan) TRACER.startSpan("benchmark", "op"); + } + } + + @Benchmark + public Object intercepted(SpanState s) { + // DDSpan.setTag delegates to the refactored DDSpanContext.setTag(String, int) + return s.span.setTag(Tags.HTTP_STATUS, 200); + } + + @Benchmark + public Object notIntercepted(SpanState s) { + return s.span.setTag("app.queue.depth", 200); + } + + static final class NoopWriter implements Writer { + @Override + public void write(List trace) {} + + @Override + public void start() {} + + @Override + public boolean flush() { + return true; + } + + @Override + public void close() {} + + @Override + public void incrementDropCounts(int spanCount) {} + } +} diff --git a/dd-trace-core/src/jmh/java/datadog/trace/core/taginterceptor/TagInterceptorBenchmark.java b/dd-trace-core/src/jmh/java/datadog/trace/core/taginterceptor/TagInterceptorBenchmark.java new file mode 100644 index 00000000000..65b25beaf80 --- /dev/null +++ b/dd-trace-core/src/jmh/java/datadog/trace/core/taginterceptor/TagInterceptorBenchmark.java @@ -0,0 +1,85 @@ +package datadog.trace.core.taginterceptor; + +import datadog.trace.api.DDTags; +import datadog.trace.bootstrap.instrumentation.api.Tags; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +/** + * Benchmark for {@link TagInterceptor#needsIntercept(String)} -- the per-tag gate {@code + * DDSpanContext} runs on every {@code setTag}. This is where the change lives: the old code + * switched over ~22 {@code String} case labels (a {@code hashCode} switch + {@code equals}); the + * new code does one open-addressed {@link datadog.trace.util.StringIndex} probe ({@code ==} fast + * path), falling to the (usually empty) split-tags set only on a miss. + * + *

Two paths, both exercised: miss (the common case -- most tags aren't intercepted, so + * the old switch fell all the way to {@code default}), and hit (an intercepted tag). + * Read-only, so {@code @Threads(8)} is clean (no span, no store to dilute the signal). Tag keys are + * interned literals -- the realistic case. + * + *

Run before (switch) vs after (StringIndex) by toggling TagInterceptor; {@code -prof gc} should + * show ~0 B/op both ways (this proves throughput, not allocation). + * + *

+ *   ./gradlew :dd-trace-core:jmh   # add -prof gc
+ * 
+ */ +@Fork(2) +@Warmup(iterations = 2) +@Measurement(iterations = 5) +@Threads(8) +@State(Scope.Benchmark) +public class TagInterceptorBenchmark { + + private final TagInterceptor interceptor = new TagInterceptor(new RuleFlags()); + + // Intercepted (fixed) tags -- handlerId hits the FIXED index. + static final String[] INTERCEPTED = { + DDTags.RESOURCE_NAME, + DDTags.SERVICE_NAME, + Tags.DB_STATEMENT, + Tags.HTTP_STATUS, + Tags.ERROR, + Tags.SPAN_KIND, + Tags.HTTP_URL, + Tags.SAMPLING_PRIORITY, + }; + + // Ordinary integration/app tags that are NOT intercepted -- the common miss path. + static final String[] NOT_INTERCEPTED = { + "http.useragent", + "db.instance", + "messaging.system", + "component", + "thread.name", + "network.peer.address", + "http.route", + "rpc.method", + }; + + /** Per-thread cursors so threads don't contend on a shared index under {@code @Threads(8)}. */ + @State(Scope.Thread) + public static class Cursor { + int hit; + int miss; + } + + @Benchmark + public boolean intercepted(Cursor cursor) { + int i = cursor.hit; + cursor.hit = (i + 1) % INTERCEPTED.length; + return interceptor.needsIntercept(INTERCEPTED[i]); + } + + @Benchmark + public boolean notIntercepted(Cursor cursor) { + int i = cursor.miss; + cursor.miss = (i + 1) % NOT_INTERCEPTED.length; + return interceptor.needsIntercept(NOT_INTERCEPTED[i]); + } +} diff --git a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java index 520311a20c1..6cb3927373f 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java @@ -906,10 +906,11 @@ public void setTag(TagMap.EntryReader entry) { return; } - // pre-check to avoid boxing + // resolve the handler once; id 0 short-circuits without a second lookup + int handlerId = tagInterceptor.handlerId(entry.tag()); boolean intercepted = - precheckIntercept(entry.tag()) - && tagInterceptor.interceptTag(this, entry.tag(), entry.objectValue()); + handlerId != 0 + && tagInterceptor.handleIntercept(this, handlerId, entry.tag(), entry.objectValue()); if (!intercepted) { synchronized (unsafeTags) { unsafeTags.set(entry); @@ -918,31 +919,11 @@ public void setTag(TagMap.EntryReader entry) { } /* - * Uses to determine if there's an opportunity to avoid primitve boxing. - * If the underlying map doesn't support efficient primitives, then boxing is used. - * If the tag may be intercepted, then boxing is also used. + * Used when handlerId determined the tag is intercepted (id != 0): the primitive must be boxed to + * pass to the interceptor. The box is reused -- the optimized TagMap caches it on store. */ - private boolean precheckIntercept(String tag) { - // Usually only a single instanceof TagMap will be loaded, - // so isOptimized is turned into a direct call and then inlines to a constant - // Since isOptimized just returns a constant - doesn't require synchronization - return !unsafeTags.isOptimized() || tagInterceptor.needsIntercept(tag); - } - - /* - * Used when precheckIntercept determines that boxing is unavoidable - * - * Either because the tagInterceptor needs to be fully checked (which requires boxing) - * In that case, a box has already been created so it makes sense to pass the box - * onto TagMap, since optimized TagMap will cache the box - * - * -- OR -- - * - * The TagMap isn't optimized and will need to box the primitive regardless of - * tag interception - */ - private void setBox(String tag, Object box) { - if (!tagInterceptor.interceptTag(this, tag, box)) { + private void setBox(int handlerId, String tag, Object box) { + if (!tagInterceptor.handleIntercept(this, handlerId, tag, box)) { synchronized (unsafeTags) { unsafeTags.set(tag, box); } @@ -953,8 +934,9 @@ public void setTag(final String tag, final boolean value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); + int handlerId = tagInterceptor.handlerId(tag); + if (handlerId != 0) { + this.setBox(handlerId, tag, value); } else { synchronized (unsafeTags) { unsafeTags.set(tag, value); @@ -966,8 +948,9 @@ public void setTag(final String tag, final int value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); + int handlerId = tagInterceptor.handlerId(tag); + if (handlerId != 0) { + this.setBox(handlerId, tag, value); } else { synchronized (unsafeTags) { unsafeTags.set(tag, value); @@ -979,10 +962,11 @@ public void setTag(final String tag, final long value) { if (null == tag) { return; } - // check needsIntercept first to avoid unnecessary boxing - boolean intercepted = - tagInterceptor.needsIntercept(tag) && tagInterceptor.interceptTag(this, tag, value); - if (!intercepted) { + // resolve once; handlerId 0 (not intercepted) stores the primitive without boxing + int handlerId = tagInterceptor.handlerId(tag); + if (handlerId != 0) { + this.setBox(handlerId, tag, value); + } else { synchronized (unsafeTags) { unsafeTags.set(tag, value); } @@ -993,8 +977,9 @@ public void setTag(final String tag, final float value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); + int handlerId = tagInterceptor.handlerId(tag); + if (handlerId != 0) { + this.setBox(handlerId, tag, value); } else { synchronized (unsafeTags) { unsafeTags.set(tag, value); @@ -1006,8 +991,9 @@ public void setTag(final String tag, final double value) { if (null == tag) { return; } - if (precheckIntercept(tag)) { - this.setBox(tag, value); + int handlerId = tagInterceptor.handlerId(tag); + if (handlerId != 0) { + this.setBox(handlerId, tag, value); } else { synchronized (unsafeTags) { unsafeTags.set(tag, value); diff --git a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java index d81a9cc8441..850002f234a 100644 --- a/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java +++ b/dd-trace-core/src/main/java/datadog/trace/core/taginterceptor/TagInterceptor.java @@ -36,21 +36,136 @@ import datadog.trace.bootstrap.instrumentation.api.URIUtils; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.core.DDSpanContext; +import datadog.trace.util.StringIndex; import java.net.URI; import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.Nullable; -public class TagInterceptor { +public final class TagInterceptor { private static final UTF8BytesString NOT_FOUND_RESOURCE_NAME = UTF8BytesString.create("404"); + // Handler ids for the intercept dispatch. 1-based: 0 is the skip sentinel ("not intercepted"), + // which lines up with the empty StringIndex slots that mapIntValues leaves at 0. + private static final int ID_RESOURCE_NAME = 1; + private static final int ID_DB_STATEMENT = 2; + private static final int ID_SERVICE = 3; // SERVICE_NAME and the legacy "service" + private static final int ID_PEER_SERVICE = 4; + private static final int ID_MANUAL_KEEP = 5; + private static final int ID_MANUAL_DROP = 6; + private static final int ID_ASM_KEEP = 7; + private static final int ID_AI_GUARD_KEEP = 8; + private static final int ID_SAMPLING_PRIORITY = 9; + private static final int ID_PROPAGATED_TRACE_SOURCE = 10; + private static final int ID_PROPAGATED_DEBUG = 11; + private static final int ID_SERVLET_CONTEXT = 12; + private static final int ID_SPAN_TYPE = 13; + private static final int ID_ANALYTICS_SAMPLE_RATE = 14; + private static final int ID_ERROR = 15; + private static final int ID_HTTP_STATUS = 16; + private static final int ID_URL_RESOURCE = 17; // HTTP_METHOD and HTTP_URL + private static final int ID_ORIGIN = 18; + private static final int ID_MEASURED = 19; + private static final int ID_SPAN_KIND = 20; + private static final int ID_SPLIT_SERVICE = 21; // a Config split-by-tag (not a fixed case) + + // Membership/dispatch index over the compile-time-fixed intercepted tags, held as the raw placed + // arrays (not a StringIndex instance) so handlerId's hot-path Support.lookup folds the refs to + // constants -- the path StringIndex recommends for hot code. Split-by-tags are Config-driven, so + // they stay a per-instance check (see handlerId) and never enter this index. + private static final int[] FIXED_HASHES; + private static final String[] FIXED_NAMES; + // Slot-aligned handler ids (built once). The name->id switch runs only at class init. + private static final int[] FIXED_HANDLER_IDS; + + static { + StringIndex.Data fixed = + StringIndex.Support.create( + DDTags.RESOURCE_NAME, + Tags.DB_STATEMENT, + DDTags.SERVICE_NAME, + "service", + Tags.PEER_SERVICE, + DDTags.MANUAL_KEEP, + DDTags.MANUAL_DROP, + Tags.ASM_KEEP, + Tags.AI_GUARD_KEEP, + Tags.SAMPLING_PRIORITY, + Tags.PROPAGATED_TRACE_SOURCE, + Tags.PROPAGATED_DEBUG, + SERVLET_CONTEXT, + SPAN_TYPE, + ANALYTICS_SAMPLE_RATE, + Tags.ERROR, + HTTP_STATUS, + HTTP_METHOD, + HTTP_URL, + ORIGIN_KEY, + MEASURED, + Tags.SPAN_KIND); + FIXED_HASHES = fixed.hashes; + FIXED_NAMES = fixed.names; + FIXED_HANDLER_IDS = + StringIndex.Support.mapIntValues(FIXED_NAMES, TagInterceptor::fixedHandlerId); + } + + private static int fixedHandlerId(String tag) { + switch (tag) { + case DDTags.RESOURCE_NAME: + return ID_RESOURCE_NAME; + case Tags.DB_STATEMENT: + return ID_DB_STATEMENT; + case DDTags.SERVICE_NAME: + case "service": + return ID_SERVICE; + case Tags.PEER_SERVICE: + return ID_PEER_SERVICE; + case DDTags.MANUAL_KEEP: + return ID_MANUAL_KEEP; + case DDTags.MANUAL_DROP: + return ID_MANUAL_DROP; + case Tags.ASM_KEEP: + return ID_ASM_KEEP; + case Tags.AI_GUARD_KEEP: + return ID_AI_GUARD_KEEP; + case Tags.SAMPLING_PRIORITY: + return ID_SAMPLING_PRIORITY; + case Tags.PROPAGATED_TRACE_SOURCE: + return ID_PROPAGATED_TRACE_SOURCE; + case Tags.PROPAGATED_DEBUG: + return ID_PROPAGATED_DEBUG; + case SERVLET_CONTEXT: + return ID_SERVLET_CONTEXT; + case SPAN_TYPE: + return ID_SPAN_TYPE; + case ANALYTICS_SAMPLE_RATE: + return ID_ANALYTICS_SAMPLE_RATE; + case Tags.ERROR: + return ID_ERROR; + case HTTP_STATUS: + return ID_HTTP_STATUS; + case HTTP_METHOD: + case HTTP_URL: + return ID_URL_RESOURCE; + case ORIGIN_KEY: + return ID_ORIGIN; + case MEASURED: + return ID_MEASURED; + case Tags.SPAN_KIND: + return ID_SPAN_KIND; + default: + return 0; // unreachable: FIXED only holds the names above + } + } + private final RuleFlags ruleFlags; private final boolean isServiceNameSetByUser; private final boolean splitByServletContext; private final String inferredServiceName; private final Set splitServiceTags; + private final boolean hasSplitTags; // short-circuits the split check when none are configured private final boolean shouldSet404ResourceName; private final boolean shouldSetUrlResourceAsName; @@ -74,6 +189,7 @@ public TagInterceptor( this.isServiceNameSetByUser = isServiceNameSetByUser; this.inferredServiceName = inferredServiceName; this.splitServiceTags = splitServiceTags; + this.hasSplitTags = !splitServiceTags.isEmpty(); this.ruleFlags = ruleFlags; splitByServletContext = splitServiceTags.contains(SERVLET_CONTEXT); @@ -100,106 +216,104 @@ public boolean needsIntercept(Map map) { } public boolean needsIntercept(String tag) { - switch (tag) { - case DDTags.RESOURCE_NAME: - case Tags.DB_STATEMENT: - case DDTags.SERVICE_NAME: - case "service": - case Tags.PEER_SERVICE: - case DDTags.MANUAL_KEEP: - case DDTags.MANUAL_DROP: - case Tags.ASM_KEEP: - case Tags.AI_GUARD_KEEP: - case Tags.SAMPLING_PRIORITY: - case Tags.PROPAGATED_TRACE_SOURCE: - case Tags.PROPAGATED_DEBUG: - case SERVLET_CONTEXT: - case SPAN_TYPE: - case ANALYTICS_SAMPLE_RATE: - case Tags.ERROR: - case HTTP_STATUS: - case HTTP_METHOD: - case HTTP_URL: - case ORIGIN_KEY: - case MEASURED: - case Tags.SPAN_KIND: - return true; + return handlerId(tag) != 0; + } - default: - return splitServiceTags.contains(tag); + /** + * Resolves {@code tag} to a dispatch handler id ({@code ID_*}), or {@code 0} when not + * intercepted. Fixed (compile-time) tags come from the static {@code FIXED_*} index; a Config + * split-by-tag -- checked only on a fixed miss -- resolves to {@link #ID_SPLIT_SERVICE}. Fixed + * wins when a tag is both, matching the original switch order. + */ + public int handlerId(String tag) { + // lookup returns the slot's id, or 0 on a miss -- which is exactly the "not fixed" sentinel + // (ids are 1-based), so no separate slot >= 0 check is needed. + int id = StringIndex.Support.lookup(FIXED_HASHES, FIXED_NAMES, FIXED_HANDLER_IDS, tag); + if (id != 0) { + return id; } + return hasSplitTags && splitServiceTags.contains(tag) ? ID_SPLIT_SERVICE : 0; } + /** + * Convenience: resolve the handler id and dispatch. {@code id == 0} short-circuits (not ours). + */ public boolean interceptTag(DDSpanContext span, String tag, Object value) { - switch (tag) { - case DDTags.RESOURCE_NAME: + int handlerId = handlerId(tag); + return handlerId != 0 && handleIntercept(span, handlerId, tag, value); + } + + public boolean handleIntercept(DDSpanContext span, int handlerId, String tag, Object value) { + switch (handlerId) { + case ID_RESOURCE_NAME: return interceptResourceName(span, value); - case Tags.DB_STATEMENT: + case ID_DB_STATEMENT: return interceptDbStatement(span, value); - case DDTags.SERVICE_NAME: - case "service": + case ID_SERVICE: return interceptServiceName(SERVICE_NAME, span, value); - case Tags.PEER_SERVICE: + case ID_PEER_SERVICE: // we still need to intercept and add this tag when the user manually set span.setTag(DDTags.PEER_SERVICE_SOURCE, Tags.PEER_SERVICE); return interceptServiceName(PEER_SERVICE, span, value); - case DDTags.MANUAL_KEEP: + case ID_MANUAL_KEEP: if (asBoolean(value)) { span.forceKeep(); return true; } return false; - case DDTags.MANUAL_DROP: + case ID_MANUAL_DROP: return interceptSamplingPriority( FORCE_MANUAL_DROP, USER_DROP, SamplingMechanism.MANUAL, span, value); - case Tags.ASM_KEEP: + case ID_ASM_KEEP: if (asBoolean(value)) { span.forceKeep(SamplingMechanism.APPSEC); return true; } return false; - case Tags.AI_GUARD_KEEP: + case ID_AI_GUARD_KEEP: if (asBoolean(value)) { span.forceKeep(SamplingMechanism.AI_GUARD); return true; } return false; - case Tags.SAMPLING_PRIORITY: + case ID_SAMPLING_PRIORITY: return interceptSamplingPriority(span, value); - case Tags.PROPAGATED_TRACE_SOURCE: + case ID_PROPAGATED_TRACE_SOURCE: if (value instanceof Integer) { span.addPropagatedTraceSource((Integer) value); return true; } return false; - case Tags.PROPAGATED_DEBUG: + case ID_PROPAGATED_DEBUG: span.updateDebugPropagation(String.valueOf(value)); return true; - case SERVLET_CONTEXT: + case ID_SERVLET_CONTEXT: return interceptServletContext(span, value); - case SPAN_TYPE: + case ID_SPAN_TYPE: return interceptSpanType(span, value); - case ANALYTICS_SAMPLE_RATE: + case ID_ANALYTICS_SAMPLE_RATE: return interceptAnalyticsSampleRate(span, value); - case Tags.ERROR: + case ID_ERROR: return interceptError(span, value); - case HTTP_STATUS: + case ID_HTTP_STATUS: // not set internally but may come from manual instrumentation return interceptHttpStatusCode(span, value); - case HTTP_METHOD: - case HTTP_URL: + case ID_URL_RESOURCE: return interceptUrlResourceAsNameRule(span, tag, value); - case ORIGIN_KEY: + case ID_ORIGIN: return interceptOrigin(span, value); - case MEASURED: + case ID_MEASURED: return interceptMeasured(span, value); - case Tags.SPAN_KIND: + case ID_SPAN_KIND: // Cache the ordinal for fast isOutbound() checks. // Return false so the value is still stored in unsafeTags for serialization. span.setSpanKindOrdinal(String.valueOf(value)); return false; + case ID_SPLIT_SERVICE: + span.setServiceName(String.valueOf(value), SPLIT_BY_TAGS); + return true; default: - return intercept(span, tag, value); + return false; } } @@ -242,14 +356,6 @@ private static void setResourceFromUrl( } } - private boolean intercept(DDSpanContext span, String tag, Object value) { - if (splitServiceTags.contains(tag)) { - span.setServiceName(String.valueOf(value), SPLIT_BY_TAGS); - return true; - } - return false; - } - private boolean interceptResourceName(DDSpanContext span, Object value) { if (ruleFlags.isEnabled(RESOURCE_NAME)) { if (null == value) {