diff --git a/ddprof-lib/src/main/cpp/sframe.cpp b/ddprof-lib/src/main/cpp/sframe.cpp index 39d1ad140..ed0de992e 100644 --- a/ddprof-lib/src/main/cpp/sframe.cpp +++ b/ddprof-lib/src/main/cpp/sframe.cpp @@ -290,7 +290,7 @@ bool SFrameParser::parse() { const SFrameFDE* fde = &fde_array[i]; if (SFRAME_FUNC_FDE_TYPE(fde->info) != 0) continue; // skip PCMASK if (fde->fre_num == 0) continue; // empty FDE - int saved_count = _count; + int saved_count = _count; // safe: addRecord capacity guard keeps _count <= INT_MAX/2 int saved_linked_frame_size = _linked_frame_size; if (!parseFDE(hdr, fde, fre_section, fre_end)) { if (_oom) return false; // OOM: partial table is not safe to use diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/ContextSetter.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/ContextSetter.java index bff497b80..5dac429dd 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/ContextSetter.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/ContextSetter.java @@ -1,13 +1,28 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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.datadoghq.profiler; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; public class ContextSetter { - private static final int TAGS_STORAGE_LIMIT = 10; private final List attributes; private final JavaProfiler profiler; @@ -15,7 +30,7 @@ public ContextSetter(JavaProfiler profiler, List attributes) { this.profiler = profiler; Set unique = new HashSet<>(attributes); this.attributes = new ArrayList<>(unique.size()); - for (int i = 0; i < Math.min(attributes.size(), TAGS_STORAGE_LIMIT); i++) { + for (int i = 0; i < Math.min(attributes.size(), ThreadContext.MAX_CUSTOM_SLOTS); i++) { String attribute = attributes.get(i); if (unique.remove(attribute)) { this.attributes.add(attribute); @@ -25,13 +40,21 @@ public ContextSetter(JavaProfiler profiler, List attributes) { public int[] snapshotTags() { int[] snapshot = new int[attributes.size()]; - snapshotTags(snapshot); + profiler.copyTags(snapshot); return snapshot; } + /** + * Copies current sidecar encodings into {@code snapshot}. The array must have at least + * {@code attributes.size()} elements; arrays shorter than {@code attributes.size()} are + * silently ignored. Indices {@code [attributes.size(), snapshot.length)} are zeroed after + * copying to prevent stale data from leaking to the caller. + * Use the no-arg {@link #snapshotTags()} overload to obtain a correctly sized array. + */ public void snapshotTags(int[] snapshot) { - if (snapshot.length <= attributes.size()) { + if (snapshot.length >= attributes.size()) { profiler.copyTags(snapshot); + Arrays.fill(snapshot, attributes.size(), snapshot.length, 0); } } @@ -50,6 +73,22 @@ public boolean setContextValue(int offset, String value) { return false; } + /** + * Re-applies attribute values from precomputed constant IDs and UTF-8 bytes, indexed by slot + * (as produced by {@link #snapshotTags(int[])}). Restores both the DD sidecar encoding and the + * OTEP attrs_data value for every slot whose constantId is {@code > 0}, in a single atomic + * publish — no String allocation, hashing, or cache lookup. Intended for re-applying + * application-managed context after a {@code setContext} span activation wipes the slots. + * + *

Partial-write on overflow. A {@code false} return does not mean the record is + * unchanged: slots that were written before an attrs_data overflow remain published. Overflowed + * slots are zeroed in both the sidecar and attrs_data views. Callers must not assume the record + * is unmodified when {@code false} is returned. + */ + public boolean setContextValuesByIdAndBytes(int[] constantIds, byte[][] utf8) { + return profiler.setContextAttributesByIdAndBytes(constantIds, utf8); + } + public boolean clearContextValue(String attribute) { return clearContextValue(offsetOf(attribute)); } diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index 33e0bdc13..9a3c7aa40 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -1,5 +1,6 @@ /* * Copyright 2018 Andrei Pangin + * Copyright 2026, Datadog, Inc * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -213,14 +214,58 @@ public void clearContext() { tlsContextStorage.get().put(0, 0, 0, 0); } + /** + * Sets a custom context attribute at the given slot offset for the current thread. + * + * @param offset slot index (0-based, in [0, 9]); out-of-range values return {@code false} + * @param value the string value to record; {@code null} returns {@code false} without + * writing; an empty string is written as a zero-length entry (not a clear — + * use {@link #clearContextAttribute(int)} to remove a value) + * @return true if the value was recorded; false if {@code offset} is out of range, + * {@code value} is null, the Dictionary is full, or {@code attrs_data} overflows + * for this slot + */ public boolean setContextAttribute(int offset, String value) { return tlsContextStorage.get().setContextAttribute(offset, value); } + /** + * Clears the custom context attribute at the given slot offset for the current thread. + * Zeros the sidecar encoding and removes it from OTEP {@code attrs_data}. + * + * @param offset slot index (0-based, in [0, 9]); out-of-range values are silently ignored + */ public void clearContextAttribute(int offset) { tlsContextStorage.get().clearContextAttribute(offset); } + /** + * Re-applies multiple custom attributes from precomputed constant IDs and UTF-8 bytes for + * the current thread in a single detach/attach window. + * + *

    + *
  • Slots with {@code constantIds[i] <= 0} are skipped.
    • + *
  • Returns {@code false} without writing if the thread's record is not currently valid + * (span-less), to avoid resurrecting a cleared record.
    • + *
  • On {@code attrs_data} overflow, the overflowed slot's sidecar is zeroed and + * {@code false} is returned; slots written before the overflow are retained.
    • + *
+ * + * @param constantIds per-slot Dictionary constant IDs; entries {@code <= 0} are skipped + * @param utf8 per-slot UTF-8 value bytes; must be non-null and at most 255 bytes + * (the OTEP attrs_data entry length field is one byte) for every slot + * whose {@code constantId > 0} + * @return true if every slot with {@code constantId > 0} was written; false on a cleared + * (span-less) record, or {@code attrs_data} overflow for any slot + * @throws NullPointerException if {@code constantIds}, {@code utf8}, or any active + * {@code utf8[i]} is null + * @throws IllegalArgumentException if the arrays have different lengths, exceed the slot limit, + * or any active {@code utf8[i]} exceeds 255 bytes + */ + public boolean setContextAttributesByIdAndBytes(int[] constantIds, byte[][] utf8) { + return tlsContextStorage.get().setContextAttributesByIdAndBytes(constantIds, utf8); + } + void copyTags(int[] snapshot) { tlsContextStorage.get().copyCustoms(snapshot); } diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/ThreadContext.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/ThreadContext.java index 349b25c13..1faffb69a 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/ThreadContext.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/ThreadContext.java @@ -19,6 +19,7 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.charset.StandardCharsets; +import java.util.Objects; /** * Thread-local context for trace/span identification. @@ -28,7 +29,7 @@ * for minimal overhead. Only little-endian platforms are supported. */ public final class ThreadContext { - private static final int MAX_CUSTOM_SLOTS = 10; + static final int MAX_CUSTOM_SLOTS = 10; // Max UTF-8 byte length for a custom attribute value. Matches the 1-byte length // field in the OTEP attrs_data entry header. Enforced up front in setContextAttribute // so replaceOtepAttribute can assume the input always fits. @@ -313,17 +314,101 @@ private boolean setContextAttributeDirect(int keyIndex, String value) { // Write both sidecar and OTEP attrs_data inside the detach/attach window // so a signal handler never sees a new sidecar encoding alongside old attrs_data. - int otepKeyIndex = keyIndex + 1; detach(); + boolean written = writeSlot(keyIndex, encoding, utf8); + attach(); + return written; + } + + /** + * Writes one slot's sidecar encoding and OTEP attrs_data value. The caller must already hold + * the detach/attach window. On attrs_data overflow the old entry was compacted out and the new + * one couldn't fit, so the sidecar is zeroed to keep both views agreeing there is no value. + * + * @return true if the value was written; false on attrs_data overflow (sidecar left zeroed) + */ + private boolean writeSlot(int keyIndex, int encoding, byte[] utf8) { ctxBuffer.putInt(TAG_ENCODINGS_OFFSET + keyIndex * Integer.BYTES, encoding); - boolean written = replaceOtepAttribute(otepKeyIndex, utf8); - if (!written) { - // attrs_data overflow: the old entry was compacted out and the new one - // couldn't fit. Zero the sidecar so both views agree there is no value. + if (!replaceOtepAttribute(keyIndex + 1, utf8)) { ctxBuffer.putInt(TAG_ENCODINGS_OFFSET + keyIndex * Integer.BYTES, 0); + return false; + } + return true; + } + + /** + * Re-applies multiple custom attributes from precomputed constant IDs and UTF-8 bytes in a + * single detach/attach window, without Dictionary lookups or per-thread cache access. + * + *

Both arrays are indexed by key slot, matching the layout produced by + * {@link #copyCustoms(int[])}. For each slot {@code i} with {@code constantIds[i] > 0}, the + * sidecar encoding (DD signal handler) and the OTEP attrs_data value (external profilers) are + * written together; slots with {@code constantIds[i] <= 0} are left untouched. Performing all + * writes inside one detach/attach window means a signal handler never observes a partially + * re-applied set. + * + *

Intended for the reapply-app-context hot path: the caller already holds the constant IDs + * (from {@link #copyCustoms(int[])}) and the UTF-8 bytes (from the original Strings), so this + * does no String allocation, hashing, or cache lookup. The record is re-published only if it + * was valid before the call, so a cleared (span-less) record is not resurrected. + * + *

Caller contract. {@code constantIds[i]} must be an ID previously returned for the + * same value via {@link #copyCustoms(int[])} within the current profiler session, and + * {@code utf8[i]} must be that value's UTF-8 bytes. The (id, bytes) pairing is not verifiable + * here; a mismatch silently diverges the sidecar and attrs_data views. + * + * @param constantIds per-slot Dictionary constant IDs; entries {@code <= 0} are skipped + * @param utf8 per-slot UTF-8 value bytes; must be non-null and at most + * {@value #MAX_VALUE_BYTES} bytes for every slot whose constantId {@code > 0} + * @return true if every slot with {@code constantId > 0} was written successfully; false if + * the record was not valid before the call (nothing is published), or if any slot + * overflowed {@code attrs_data} (that slot's sidecar is zeroed). Note: a {@code false} + * return due to {@code attrs_data} overflow does not mean the record is + * unmodified — slots processed before the overflowed one are durably written. + * @throws NullPointerException if {@code constantIds}, {@code utf8}, or any active + * {@code utf8[i]} (where {@code constantIds[i] > 0}) is null + * @throws IllegalArgumentException if the arrays have different lengths, + * {@code constantIds.length > MAX_CUSTOM_SLOTS}, or any + * active {@code utf8[i].length > MAX_VALUE_BYTES} + */ + public boolean setContextAttributesByIdAndBytes(int[] constantIds, byte[][] utf8) { + Objects.requireNonNull(constantIds, "constantIds"); + Objects.requireNonNull(utf8, "utf8"); + if (constantIds.length != utf8.length) { + throw new IllegalArgumentException("constantIds and utf8 must have the same length"); + } + if (constantIds.length > MAX_CUSTOM_SLOTS) { + throw new IllegalArgumentException("constantIds.length exceeds MAX_CUSTOM_SLOTS"); + } + int len = constantIds.length; + // Validate active slots before touching the buffer so a bad input never leaves + // the record detached (valid=0) after an exception unwinds past attach(). + for (int i = 0; i < len; i++) { + if (constantIds[i] > 0) { + byte[] bytes = Objects.requireNonNull(utf8[i], "utf8[" + i + "]"); + if (bytes.length > MAX_VALUE_BYTES) { + throw new IllegalArgumentException("utf8[" + i + "].length exceeds MAX_VALUE_BYTES"); + } + } + } + // Never resurrect a cleared (span-less) record: valid=0 means no reader can observe + // what we write, and re-publishing would expose a record with no trace/span context. + if (ctxBuffer.get(validOffset) == 0) { + return false; + } + detach(); + boolean allWritten = true; + for (int i = 0; i < len; i++) { + int constantId = constantIds[i]; + if (constantId <= 0) { + continue; + } + if (!writeSlot(i, constantId, utf8[i])) { + allWritten = false; + } } attach(); - return written; + return allWritten; } /** diff --git a/ddprof-stresstest/build.gradle.kts b/ddprof-stresstest/build.gradle.kts index 6cdcfe38b..8119efb61 100644 --- a/ddprof-stresstest/build.gradle.kts +++ b/ddprof-stresstest/build.gradle.kts @@ -1,3 +1,18 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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. + */ import com.datadoghq.native.util.PlatformUtils plugins { @@ -67,6 +82,9 @@ dependencies { // dd-trace-api: annotations only at compile time. The patched dd-java-agent // provides the (relocated) runtime classes and intercepts @Trace. "chaosCompileOnly"(libs.dd.trace.api) + // ddprof-lib public API: compile-only; the patched dd-java-agent provides the + // classes at runtime for antagonists that call JavaProfiler/ThreadContext directly. + "chaosCompileOnly"(project(mapOf("path" to ":ddprof-lib", "configuration" to "debug"))) } tasks.register("chaosJar") { diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java index 802d1fc2e..9b2cccc43 100644 --- a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java @@ -90,6 +90,8 @@ private static Antagonist create(String name) { return new WeakRefWaveAntagonist(); case "dump-storm": return new DumpStormAntagonist(); + case "reapply-context": + return new ReapplyContextAntagonist(); // Deferred: dlopen-churn (needs per-arch dummy .so built in CI prep). default: throw new IllegalArgumentException("unknown antagonist: " + name); diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextAntagonist.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextAntagonist.java new file mode 100644 index 000000000..e853fa3c8 --- /dev/null +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReapplyContextAntagonist.java @@ -0,0 +1,124 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + */ +package com.datadoghq.profiler.chaos; + +import com.datadoghq.profiler.ContextSetter; +import com.datadoghq.profiler.JavaProfiler; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.Arrays; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +/** + * Drives the reapply-by-id-and-bytes hot path continuously from multiple threads while the + * profiler's wall-clock signal fires, racing the {@code detach}/{@code attach} window. + * + *

Each worker thread loops: span-activate (wiping slots) → reapply snapshot. This mirrors + * dd-trace-java's {@code reapplyAppContext} pattern and exercises the single-window invariant + * (no partial publish visible to a signal handler) under high thread count and signal pressure. + * + *

The only expected failure signal is a JVM crash. An unexpected {@code false} return from + * reapply (which should never happen since the record is always valid right after + * {@code setContext}) throws {@link IllegalStateException} to surface the bug immediately. + */ +public final class ReapplyContextAntagonist implements Antagonist { + + private static final String[] ATTR_NAMES = { + "http.route", "http.method", "http.status", "db.operation", "rpc.service" + }; + + private static final String[] ROUTES = { + "GET /api/users", + "POST /api/orders", + "GET /api/health", + "PUT /api/users/{id}", + "DELETE /api/sessions" + }; + + private final int workerCount; + private final ExecutorService pool; + private volatile boolean running; + + public ReapplyContextAntagonist() { + this(8); + } + + public ReapplyContextAntagonist(int workerCount) { + this.workerCount = workerCount; + this.pool = + Executors.newFixedThreadPool( + workerCount, + r -> { + Thread t = new Thread(r, "chaos-reapply-context"); + t.setDaemon(true); + return t; + }); + } + + @Override + public String name() { + return "reapply-context"; + } + + @Override + public void start() { + running = true; + for (int i = 0; i < workerCount; i++) { + pool.submit(this::workerLoop); + } + } + + @Override + public void stopGracefully(Duration timeout) { + running = false; + pool.shutdown(); + try { + pool.awaitTermination(timeout.toMillis(), TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private void workerLoop() { + JavaProfiler profiler; + try { + profiler = JavaProfiler.getInstance(); + } catch (Exception e) { + System.err.println("[chaos] reapply-context: failed to get profiler: " + e); + return; + } + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList(ATTR_NAMES)); + + // Prime the per-thread encoding cache and capture a stable snapshot. + long spanId = Thread.currentThread().getId() + 1; + long localRootSpanId = spanId * 31L; + long traceIdLow = spanId * 6364136223846793005L + 1442695040888963407L; + profiler.setContext(localRootSpanId, spanId, 0, traceIdLow); + for (int i = 0; i < ROUTES.length; i++) { + contextSetter.setContextValue(i, ROUTES[i]); + } + int[] constantIds = contextSetter.snapshotTags(); + byte[][] utf8 = new byte[ROUTES.length][]; + for (int i = 0; i < ROUTES.length; i++) { + utf8[i] = ROUTES[i].getBytes(StandardCharsets.UTF_8); + } + + while (running) { + // Span activation wipes all custom slots. + profiler.setContext(localRootSpanId, spanId, 0, traceIdLow); + // Reapply restores them — this is the hot path under test. + if (!contextSetter.setContextValuesByIdAndBytes(constantIds, utf8)) { + throw new IllegalStateException("reapply failed unexpectedly — record should be valid after setContext"); + } + } + } +} diff --git a/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/stresstest/scenarios/throughput/ThreadContextBenchmark.java b/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/stresstest/scenarios/throughput/ThreadContextBenchmark.java index 25c896007..dbd4eec4c 100644 --- a/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/stresstest/scenarios/throughput/ThreadContextBenchmark.java +++ b/ddprof-stresstest/src/jmh/java/com/datadoghq/profiler/stresstest/scenarios/throughput/ThreadContextBenchmark.java @@ -17,6 +17,7 @@ import com.datadoghq.profiler.JavaProfiler; import com.datadoghq.profiler.ThreadContext; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.concurrent.ThreadLocalRandom; @@ -71,6 +72,7 @@ public static class ThreadState { ThreadContext ctx; long spanId; long localRootSpanId; + long traceIdLow; int counter; @Setup(Level.Trial) @@ -78,12 +80,57 @@ public void setup(ProfilerState ps) { ctx = ps.profiler.getThreadContext(); spanId = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); localRootSpanId = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + traceIdLow = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + } + } + + @State(Scope.Thread) + public static class ReapplyState { + ThreadContext ctx; + long spanId; + long localRootSpanId; + long traceIdLow; + int[] constantIds; + byte[][] utf8; + + @Setup(Level.Trial) + public void setup(ProfilerState ps) { + ctx = ps.profiler.getThreadContext(); + spanId = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + localRootSpanId = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + traceIdLow = ThreadLocalRandom.current().nextLong(1, Long.MAX_VALUE); + + // Prime the normal path to obtain constant IDs, then snapshot for reapply. + ctx.put(localRootSpanId, spanId, 0, traceIdLow); + for (int i = 0; i < ROUTES.length; i++) { + ctx.setContextAttribute(i, ROUTES[i]); + } + constantIds = new int[10]; + ctx.copyCustoms(constantIds); + utf8 = new byte[10][]; + for (int i = 0; i < ROUTES.length; i++) { + utf8[i] = ROUTES[i].getBytes(StandardCharsets.UTF_8); + } + } + + @Setup(Level.Iteration) + public void resetToSteadyState() { + // Re-establish a live span (valid=1) and pre-populate attrs_data with all slots + // before each measurement window. Without this, reapplyByIdAndBytes sees a + // different attrs_data state on the first invocation of each iteration (empty + // after put() in the previous iteration or after the trial setup), causing a + // bimodal distribution across forks due to JIT profile divergence. + ctx.put(localRootSpanId, spanId, 0, traceIdLow); + if (!ctx.setContextAttributesByIdAndBytes(constantIds, utf8)) { + throw new IllegalStateException( + "resetToSteadyState: setContextAttributesByIdAndBytes failed; benchmark state invalid"); + } } } @Benchmark public void setContextFull(ThreadState ts) { - ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.spanId); + ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.traceIdLow); } @Benchmark @@ -93,33 +140,33 @@ public boolean setAttrCacheHit(ThreadState ts) { @Benchmark public void spanLifecycle(ThreadState ts) { - ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.spanId); + ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.traceIdLow); ts.ctx.setContextAttribute(0, ROUTES[ts.counter++ % ROUTES.length]); } @Benchmark @Threads(2) public void setContextFull_2t(ThreadState ts) { - ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.spanId); + ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.traceIdLow); } @Benchmark @Threads(4) public void setContextFull_4t(ThreadState ts) { - ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.spanId); + ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.traceIdLow); } @Benchmark @Threads(2) public void spanLifecycle_2t(ThreadState ts) { - ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.spanId); + ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.traceIdLow); ts.ctx.setContextAttribute(0, ROUTES[ts.counter++ % ROUTES.length]); } @Benchmark @Threads(4) public void spanLifecycle_4t(ThreadState ts) { - ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.spanId); + ts.ctx.put(ts.localRootSpanId, ts.spanId, 0, ts.traceIdLow); ts.ctx.setContextAttribute(0, ROUTES[ts.counter++ % ROUTES.length]); } @@ -132,4 +179,31 @@ public long getSpanId(ThreadState ts) { public void clearContext(ThreadState ts) { ts.ctx.put(0, 0, 0, 0); } + + /** Bare reapply cost with constant IDs and bytes already in hand — no Dictionary lookup. */ + @Benchmark + public boolean reapplyByIdAndBytes(ReapplyState rs) { + return rs.ctx.setContextAttributesByIdAndBytes(rs.constantIds, rs.utf8); + } + + /** Full reapply cycle: span activation wipes slots, then reapply restores them. */ + @Benchmark + public boolean reapplyCycle(ReapplyState rs) { + rs.ctx.put(rs.localRootSpanId, rs.spanId, 0, rs.spanId); + return rs.ctx.setContextAttributesByIdAndBytes(rs.constantIds, rs.utf8); + } + + @Benchmark + @Threads(2) + public boolean reapplyCycle_2t(ReapplyState rs) { + rs.ctx.put(rs.localRootSpanId, rs.spanId, 0, rs.spanId); + return rs.ctx.setContextAttributesByIdAndBytes(rs.constantIds, rs.utf8); + } + + @Benchmark + @Threads(4) + public boolean reapplyCycle_4t(ReapplyState rs) { + rs.ctx.put(rs.localRootSpanId, rs.spanId, 0, rs.spanId); + return rs.ctx.setContextAttributesByIdAndBytes(rs.constantIds, rs.utf8); + } } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/context/TagContextTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/context/TagContextTest.java index 9b8fe4404..95c8fecd5 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/context/TagContextTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/context/TagContextTest.java @@ -1,5 +1,21 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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.datadoghq.profiler.context; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.ArrayList; import java.util.HashMap; @@ -13,10 +29,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junitpioneer.jupiter.RetryingTest; import org.openjdk.jmc.common.item.IItem; @@ -34,9 +52,16 @@ public class TagContextTest extends AbstractProfilerTest { + @BeforeEach + void assumeNotJ9() { + // On J9, ProfiledThread (and thus the OTEP TLS buffer) is not allocated until the thread + // is registered for wall-clock profiling, so initializeContextTLS0() returns null and + // ThreadContext creation throws. These tests require a live ThreadContext from the start. + Assumptions.assumeTrue(!Platform.isJ9()); + } + @RetryingTest(10) public void test() throws InterruptedException { - Assumptions.assumeTrue(!Platform.isJ9()); registerCurrentThreadForWallClockProfiling(); ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2", "tag1")); @@ -156,7 +181,6 @@ private String readTag(ContextSetter contextSetter, String tag) { @Test public void testSnapshotRestore() throws Exception { // J9 does not initialize ThreadContext for non-profiled threads; skip. - Assumptions.assumeTrue(!Platform.isJ9()); registerCurrentThreadForWallClockProfiling(); ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); @@ -189,7 +213,6 @@ public void testSnapshotRestore() throws Exception { @Test public void testAttrsDataOverflow() throws Exception { - Assumptions.assumeTrue(!Platform.isJ9()); registerCurrentThreadForWallClockProfiling(); List attrs = new ArrayList<>(); for (int i = 1; i <= 10; i++) { @@ -213,7 +236,6 @@ public void testAttrsDataOverflow() throws Exception { @Test public void testPutClearsCustomSlots() throws Exception { - Assumptions.assumeTrue(!Platform.isJ9()); registerCurrentThreadForWallClockProfiling(); ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); @@ -222,13 +244,12 @@ public void testPutClearsCustomSlots() throws Exception { // setContext() triggers setContextDirect which resets attrs_data_size to the LRS entry only, // dropping all user attribute entries — so scanning attrs_data for tag1 returns null. - profiler.setContext(1L, 42L, 0L, 42L); + profiler.setContext(1L, 42L, 0L, 43L); assertNull(readTag(contextSetter, "tag1"), "tag1 must be null after setContext resets attrs_data"); } @Test public void testCrossSlotIsolation() throws Exception { - Assumptions.assumeTrue(!Platform.isJ9()); registerCurrentThreadForWallClockProfiling(); ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); @@ -239,6 +260,359 @@ public void testCrossSlotIsolation() throws Exception { assertNull(readTag(contextSetter, "tag2")); } + @Test + public void testReapplyByIdAndBytes() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + int slot = contextSetter.offsetOf("tag1"); + String value = "app-managed"; + + // Set the attribute the normal way, then capture both the constant ID (sidecar) and the + // UTF-8 bytes — exactly what dd-trace-java retains for the reapply hot path. + assertTrue(contextSetter.setContextValue("tag1", value)); + int[] ids = contextSetter.snapshotTags(); + int savedId = ids[slot]; + assertNotEquals(0, savedId); + byte[][] bytes = new byte[ids.length][]; + bytes[slot] = value.getBytes(StandardCharsets.UTF_8); + + // setContext (span activation) wipes both views. + profiler.setContext(1L, 42L, 0L, 43L); + assertNull(readTag(contextSetter, "tag1"), "attrs_data must be wiped by setContext"); + assertEquals(0, contextSetter.snapshotTags()[slot], "sidecar must be wiped by setContext"); + + // Reapply by ID + bytes restores BOTH views. + assertTrue(contextSetter.setContextValuesByIdAndBytes(ids, bytes)); + assertEquals(value, readTag(contextSetter, "tag1"), "attrs_data must be restored"); + assertEquals(savedId, contextSetter.snapshotTags()[slot], "sidecar must be restored"); + } + + @Test + public void testReapplyByIdAndBytesRejectsBadArgs() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + int slot = contextSetter.offsetOf("tag1"); + + assertTrue(contextSetter.setContextValue("tag1", "v")); + int id = contextSetter.snapshotTags()[slot]; + assertNotEquals(0, id); + + // Null arrays and length mismatch must throw. + assertThrows(NullPointerException.class, + () -> contextSetter.setContextValuesByIdAndBytes(null, new byte[1][])); + assertThrows(NullPointerException.class, + () -> contextSetter.setContextValuesByIdAndBytes(new int[1], null)); + assertThrows(IllegalArgumentException.class, + () -> contextSetter.setContextValuesByIdAndBytes(new int[2], new byte[3][])); + + // A slot with constantId > 0 requires non-null bytes within the size limit. + assertThrows(NullPointerException.class, + () -> contextSetter.setContextValuesByIdAndBytes( + new int[] {id, 0}, new byte[][] {null, null})); + // Record must remain attached (valid=1) after the exception — no detach leak. + assertEquals(id, contextSetter.snapshotTags()[slot], + "sidecar must be unchanged after NPE on active utf8[i]"); + + assertThrows(IllegalArgumentException.class, + () -> contextSetter.setContextValuesByIdAndBytes( + new int[] {id, 0}, new byte[][] {new byte[256], null})); + // Record must remain attached (valid=1) after the exception — no detach leak. + assertEquals(id, contextSetter.snapshotTags()[slot], + "sidecar must be unchanged after IAE on oversized utf8[i]"); + + // 255 bytes is the boundary and must be accepted. + // Register the 255-byte value so its constant ID matches the bytes we pass. + byte[] ok255 = new byte[255]; + Arrays.fill(ok255, (byte) 'x'); + assertTrue(contextSetter.setContextValue("tag1", new String(ok255, StandardCharsets.UTF_8))); + int id255 = contextSetter.snapshotTags()[slot]; + assertNotEquals(0, id255); + assertTrue(contextSetter.setContextValuesByIdAndBytes( + new int[] {id255, 0}, new byte[][] {ok255, null})); + } + + @Test + public void testReapplyByIdAndBytesReplacesExistingValue() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + int slot = contextSetter.offsetOf("tag1"); + + // Capture the ID + bytes for "first". + assertTrue(contextSetter.setContextValue("tag1", "first")); + int[] idsFirst = contextSetter.snapshotTags(); + byte[][] bytesFirst = new byte[idsFirst.length][]; + bytesFirst[slot] = "first".getBytes(StandardCharsets.UTF_8); + + // Overwrite the live slot with a different value. + assertTrue(contextSetter.setContextValue("tag1", "second")); + assertEquals("second", readTag(contextSetter, "tag1")); + + // Reapply "first" by ID + bytes over the live "second" — exercises the + // compact-then-insert path in replaceOtepAttribute. + assertTrue(contextSetter.setContextValuesByIdAndBytes(idsFirst, bytesFirst)); + assertEquals("first", readTag(contextSetter, "tag1")); + assertEquals(idsFirst[slot], contextSetter.snapshotTags()[slot]); + } + + @Test + public void testReapplyByIdAndBytesAfterClear() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + int slot = contextSetter.offsetOf("tag1"); + + assertTrue(contextSetter.setContextValue("tag1", "live")); + int[] ids = contextSetter.snapshotTags(); + byte[][] bytes = new byte[ids.length][]; + bytes[slot] = "live".getBytes(StandardCharsets.UTF_8); + + assertTrue(contextSetter.clearContextValue("tag1")); + assertNull(readTag(contextSetter, "tag1")); + assertEquals(0, contextSetter.snapshotTags()[slot]); + + assertTrue(contextSetter.setContextValuesByIdAndBytes(ids, bytes)); + assertEquals("live", readTag(contextSetter, "tag1")); + assertEquals(ids[slot], contextSetter.snapshotTags()[slot]); + } + + @Test + public void testReapplyByIdAndBytesClearedRecord() throws Exception { + // Verifies that setContextValuesByIdAndBytes never resurrects a cleared (span-less) record. + // A cleared record has valid=0 and no trace/span context; re-publishing it would expose + // attribute values with no associated trace, which is meaningless to the signal handler. + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + int slot = contextSetter.offsetOf("tag1"); + + // Establish a live record with tag1 set. + profiler.setContext(1L, 42L, 0L, 43L); + assertTrue(contextSetter.setContextValue("tag1", "will-be-cleared")); + int[] ids = contextSetter.snapshotTags(); + byte[][] bytes = new byte[ids.length][]; + bytes[slot] = "will-be-cleared".getBytes(StandardCharsets.UTF_8); + + // Drive valid=0 via the all-zero clear path (clearContext → put(0,0,0,0) → no attach()). + profiler.clearContext(); + // readContextAttribute respects valid=0 and returns null, confirming the record is dark. + assertNull(readTag(contextSetter, "tag1")); + + // Reapply must return false and must not resurrect the cleared record. + assertFalse(contextSetter.setContextValuesByIdAndBytes(ids, bytes), + "setContextValuesByIdAndBytes must return false when the record is cleared (valid=0)"); + assertNull(readTag(contextSetter, "tag1"), + "cleared record must not be resurrected by setContextValuesByIdAndBytes"); + } + + @Test + public void testReapplyByIdAndBytesOverflowRollback() throws Exception { + registerCurrentThreadForWallClockProfiling(); + List attrs = new ArrayList<>(); + for (int i = 1; i <= 10; i++) { + attrs.add("tag" + i); + } + ContextSetter contextSetter = new ContextSetter(profiler, attrs); + + // Register one 255-byte value to obtain a valid constant ID. + char[] chars = new char[255]; + Arrays.fill(chars, 'x'); + String bigValue = new String(chars); + assertTrue(contextSetter.setContextValue("tag1", bigValue)); + int bigId = contextSetter.snapshotTags()[contextSetter.offsetOf("tag1")]; + assertNotEquals(0, bigId); + byte[] bigBytes = bigValue.getBytes(StandardCharsets.UTF_8); + + // Reapply the same 255-byte value to all 10 slots — attrs_data cannot hold them all. + int[] ids = new int[10]; + byte[][] bytes = new byte[10][]; + Arrays.fill(ids, bigId); + Arrays.fill(bytes, bigBytes); + assertFalse(contextSetter.setContextValuesByIdAndBytes(ids, bytes), + "10 x 255-byte values must overflow attrs_data"); + + // The last slot certainly overflowed: its sidecar must be zeroed and attrs_data empty. + int lastSlot = contextSetter.offsetOf("tag10"); + assertEquals(0, contextSetter.snapshotTags()[lastSlot], + "overflowed slot's sidecar must be zeroed"); + assertNull(readTag(contextSetter, "tag10"), + "overflowed slot must read null — the entry never landed in attrs_data"); + + // Slots processed before the overflow are durably written — false does not mean + // the record is unchanged. At least tag1 (slot 0) must retain the new value. + int firstSlot = contextSetter.offsetOf("tag1"); + assertEquals(bigId, contextSetter.snapshotTags()[firstSlot], + "slot 0 processed before overflow must have its sidecar durably written"); + assertEquals(bigValue, readTag(contextSetter, "tag1"), + "slot 0 processed before overflow must be readable via attrs_data"); + } + + // ----------------------------------------------------------------------- + // Acceptance tests for the MAX_CUSTOM_SLOTS guard fixes + // ----------------------------------------------------------------------- + + /** + * Test 1: setContextValuesByIdAndBytes must throw IllegalArgumentException immediately when + * the arrays are longer than MAX_CUSTOM_SLOTS (10), and must not perform + * any partial write before the rejection. + */ + @Test + public void testSetContextValuesByIdAndBytesRejectsArraysLongerThanMaxSlots() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + int slot = contextSetter.offsetOf("tag1"); + + // Establish a known value and capture its constant ID. + assertTrue(contextSetter.setContextValue("tag1", "original")); + int savedId = contextSetter.snapshotTags()[slot]; + assertNotEquals(0, savedId); + + // Build arrays of length 11 (> MAX_CUSTOM_SLOTS = 10). + int[] ids = new int[11]; + byte[][] utf8 = new byte[11][]; + ids[0] = savedId; + utf8[0] = "original".getBytes(StandardCharsets.UTF_8); + // All other entries remain 0 / null. + + // The call must be rejected with an exception before any write. + assertThrows(IllegalArgumentException.class, + () -> contextSetter.setContextValuesByIdAndBytes(ids, utf8), + "setContextValuesByIdAndBytes must throw when array length > MAX_CUSTOM_SLOTS"); + + // No partial write: the sidecar for slot 0 must be unchanged. + assertEquals(savedId, contextSetter.snapshotTags()[slot], + "sidecar must not be modified before the length guard fires"); + } + + /** + * Test 2: setContextValuesByIdAndBytes must accept arrays of exactly + * MAX_CUSTOM_SLOTS (10) and return true, restoring all sidecar values. + */ + @Test + public void testSetContextValuesByIdAndBytesAcceptsExactlyMaxSlots() throws Exception { + registerCurrentThreadForWallClockProfiling(); + List attrs = new ArrayList<>(); + for (int i = 1; i <= 10; i++) { + attrs.add("tag" + i); + } + ContextSetter contextSetter = new ContextSetter(profiler, attrs); + + // Set all 10 attributes to distinct values and capture constant IDs + bytes. + int[] savedIds = new int[10]; + byte[][] savedBytes = new byte[10][]; + for (int i = 0; i < 10; i++) { + String value = "val" + i; + assertTrue(contextSetter.setContextValue("tag" + (i + 1), value)); + savedBytes[i] = value.getBytes(StandardCharsets.UTF_8); + } + int[] snapshot = contextSetter.snapshotTags(); + for (int i = 0; i < 10; i++) { + savedIds[i] = snapshot[i]; + assertNotEquals(0, savedIds[i], "tag" + (i + 1) + " must have a non-zero sidecar ID"); + } + + // Wipe all slots via setContext (span activation). + profiler.setContext(1L, 42L, 0L, 43L); + + // Reapply with exactly-10-element arrays — must succeed. + assertTrue(contextSetter.setContextValuesByIdAndBytes(savedIds, savedBytes), + "setContextValuesByIdAndBytes must return true for arrays of length == MAX_CUSTOM_SLOTS"); + + // All 10 sidecar IDs must be restored. + int[] restored = contextSetter.snapshotTags(); + for (int i = 0; i < 10; i++) { + assertEquals(savedIds[i], restored[i], + "sidecar for tag" + (i + 1) + " must be restored after reapply"); + } + } + + /** + * Test 3: snapshotTags(int[]) with an oversized buffer (length > attributes.size()) + * must write the managed indices [0, attributes.size()) with the current sidecar values, + * and zero out the extra indices [attributes.size(), snapshot.length). + */ + @Test + public void testSnapshotTagsOversizedBufferCopiesAndZerosExtras() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + + assertTrue(contextSetter.setContextValue("tag1", "v1")); + assertTrue(contextSetter.setContextValue("tag2", "v2")); + + // Verify no-arg overload returns valid IDs. + int[] canonical = contextSetter.snapshotTags(); + assertNotEquals(0, canonical[0]); + assertNotEquals(0, canonical[1]); + + // Oversized buffer: length 5 > attributes.size() == 2. + int[] oversized = new int[5]; + Arrays.fill(oversized, -1); + contextSetter.snapshotTags(oversized); + + // Managed indices [0, attributes.size()) must contain the current sidecar values. + assertEquals(canonical[0], oversized[0], + "oversized buffer[0] must match no-arg snapshotTags()[0]"); + assertEquals(canonical[1], oversized[1], + "oversized buffer[1] must match no-arg snapshotTags()[1]"); + + // Extra indices [attributes.size(), snapshot.length) must be zeroed. + for (int i = 2; i < oversized.length; i++) { + assertEquals(0, oversized[i], + "oversized buffer element [" + i + "] must be zeroed by snapshotTags"); + } + + // No-arg overload must still work correctly. + int[] check = contextSetter.snapshotTags(); + assertEquals(canonical[0], check[0]); + assertEquals(canonical[1], check[1]); + } + + /** + * Test 4: snapshotTags(int[]) with an undersized buffer (length < attributes.size()) + * must be a no-op — existing no-op semantics must be preserved. + */ + @Test + public void testSnapshotTagsUndersizedBufferIsNoOp() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2", "tag3")); + + assertTrue(contextSetter.setContextValue("tag1", "a")); + assertTrue(contextSetter.setContextValue("tag2", "b")); + assertTrue(contextSetter.setContextValue("tag3", "c")); + + // Undersized buffer: length 1 < attributes.size() == 3. + int[] undersized = new int[1]; + undersized[0] = -1; + contextSetter.snapshotTags(undersized); + + assertEquals(-1, undersized[0], + "undersized buffer must not be written by snapshotTags"); + } + + /** + * Test 5: snapshotTags(int[]) with an exact-size buffer (length == attributes.size()) + * must copy the current sidecar values correctly. + */ + @Test + public void testSnapshotTagsExactSizeBufferCopiesCorrectly() throws Exception { + registerCurrentThreadForWallClockProfiling(); + ContextSetter contextSetter = new ContextSetter(profiler, Arrays.asList("tag1", "tag2")); + + assertTrue(contextSetter.setContextValue("tag1", "x")); + assertTrue(contextSetter.setContextValue("tag2", "y")); + + // No-arg overload to obtain expected values. + int[] canonical = contextSetter.snapshotTags(); + assertNotEquals(0, canonical[0]); + assertNotEquals(0, canonical[1]); + + // Exact-size buffer: length 2 == attributes.size() == 2. + int[] exact = new int[2]; + contextSetter.snapshotTags(exact); + + assertEquals(canonical[0], exact[0], + "exact-size buffer[0] must match no-arg snapshotTags()[0]"); + assertEquals(canonical[1], exact[1], + "exact-size buffer[1] must match no-arg snapshotTags()[1]"); + } + private void work(ContextSetter contextSetter, String contextAttribute, String contextValue) throws InterruptedException { assertTrue(contextSetter.setContextValue(contextAttribute, contextValue)); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/BaseContextWallClockTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/BaseContextWallClockTest.java index 2c9f5e464..1ff6bbb7c 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/BaseContextWallClockTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/wallclock/BaseContextWallClockTest.java @@ -130,7 +130,13 @@ void test(AbstractProfilerTest test, boolean assertContext, String cstack) throw method2Weight += weight; attributed = true; } else if (stackTrace.contains("method1Impl") - && !stackTrace.contains("method2") && !stackTrace.contains("method3")) { + && !stackTrace.contains("method2") && !stackTrace.contains("method3") + && !stackTrace.contains("Object.wait")) { + // Exclude Object.wait frames: while method1Impl is blocked in monitor.wait(), + // method3 runs concurrently on the executor thread. The wall-clock profiler + // samples all threads, so that same window produces method3Weight samples on + // the executor AND method1Weight samples on the main thread. Counting the + // main-thread double-dip inflates method1's share to ~40-55% instead of ~33%. if (assertContext) { // need to check this after method2 because method1 calls method2 // it's the root so spanId == rootSpanId @@ -170,17 +176,12 @@ void test(AbstractProfilerTest test, boolean assertContext, String cstack) throw // it is under investigation but until it gets resolved we will just relax the error margin double allowedError = Platform.isAarch64() && "BellSoft".equals(System.getProperty("java.vendor")) ? 0.4d : 0.2d; - // After async-profiler 4.2.1 integration and wall clock collapsing fixes, weight - // distribution changed across all unwinding modes (vm, vmx, fp, dwarf). All modes now - // show ~55% weight for method1Impl instead of expected ~33%. Root causes include: - // 1. DWARF: collects 10-20 native frames (vs 2-5 for FP), native frame PCs vary causing - // trace ID fragmentation - // 2. FP/VMX: async-profiler integration changed frame collection or attribution behavior - // 3. All modes: trace IDs hash all frames including native PCs with slight address variations - // Proper fix requires architectural changes (hash only Java frames or normalize native PCs - // to function entry points). For now, relax tolerance to acknowledge observed behavior. + // DWARF collects 10-20 native frames per sample (vs 2-5 for FP/VM). Those native PCs vary + // slightly between samples, fragmenting trace IDs and causing some method2/method3 samples + // to be lost or misattributed. The 0.3 tolerance accommodates that fragmentation without + // masking genuine context-attribution bugs. if (cstack != null && (cstack.equals("vm") || cstack.equals("dwarf") || cstack.equals("fp") || cstack.equals("vmx"))) { - allowedError = 0.3d; // Allow up to 30% deviation for affected modes + allowedError = 0.3d; } // context filtering should prevent these @@ -190,11 +191,11 @@ void test(AbstractProfilerTest test, boolean assertContext, String cstack) throw // still useful to run the profiler, though - so just skipping the assertions here if (config.equals("release") || config.equals("debug")) { double totalWeight = method1Weight + method2Weight + method3Weight + unattributedWeight; - // method1 has ~50% self time, 50% calling method2 + // Each method sleeps for the same duration (10ms), so each should contribute ~33%. + // method1Impl's monitor.wait time is excluded from method1Weight (see attribution above) + // to measure self-time only, not elapsed time waiting for method3. assertWeight("method1Impl", totalWeight, method1Weight, 0.33, allowedError); - // method2 has as much self time as method1 assertWeight("method2Impl", totalWeight, method2Weight, 0.33, allowedError); - // method3 has as much self time as method1, and should account for half the executor's thread's time assertWeight("method3Impl", totalWeight, method3Weight, 0.33, allowedError); // The recording captures counter values before the final cleanup (before processTraces // runs and frees all traces). Verify the recording contains meaningful data.