From 00b5aafa6d54796e73f9a4932db663a1604cef19 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 13 Jul 2026 15:57:51 +0200 Subject: [PATCH 1/2] Reduce line-level coverage recording overhead with a per-method probe-array swap Swap Jacoco's shared probe array for a per-test array once at method entry, so Jacoco's native probes record per-test coverage with no per-probe callback. Aggregate coverage is preserved by OR-ing per-test bits back into the shared array at report time. Excludes the CoverageProbes interface (whose only bytecode is the new fallback default) from the coverage check, consistent with the rest of the coverage-infra classes. Co-Authored-By: Claude Opus 4.8 --- .../CiVisibilityInstrumentationTest.groovy | 3 - .../civisibility/CiVisibilitySystem.java | 2 - .../SkippableAwareCoverageStoreFactory.java | 5 -- .../coverage/file/FileCoverageStore.java | 5 -- .../coverage/file/FileProbes.java | 5 -- .../coverage/line/ExecutionDataAdapter.java | 38 ++++++++-- .../coverage/line/LineCoverageStore.java | 16 ++--- .../coverage/line/LineProbes.java | 16 ++--- .../line/ExecutionDataAdapterTest.java | 71 +++++++++++++++++++ .../coverage/line/LineCoverageStoreTest.java | 69 ++++++++++++++++++ .../coverage/line/LineProbesTest.java | 67 +++++++++++++++++ .../ClassInstrumenterInstrumentation.java | 57 --------------- .../jacoco/MethodVisitorWrapper.java | 7 ++ .../jacoco/ProbeInserterInstrumentation.java | 40 +++++------ internal-api/build.gradle.kts | 1 + .../coverage/CoveragePerTestBridge.java | 65 +---------------- .../civisibility/coverage/CoverageProbes.java | 15 +++- .../civisibility/coverage/CoverageStore.java | 6 +- .../coverage/NoOpCoverageStore.java | 5 -- .../api/civisibility/coverage/NoOpProbes.java | 3 - 20 files changed, 298 insertions(+), 198 deletions(-) create mode 100644 dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java create mode 100644 dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java create mode 100644 dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java delete mode 100644 dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java diff --git a/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy b/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy index e71a8a5d8c9..2d2a8261d64 100644 --- a/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy +++ b/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy @@ -15,7 +15,6 @@ import datadog.trace.api.civisibility.config.LibraryCapability import datadog.trace.api.civisibility.config.TestFQN import datadog.trace.api.civisibility.config.TestIdentifier import datadog.trace.api.civisibility.config.TestMetadata -import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge import datadog.trace.api.civisibility.events.TestEventsHandler import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector import datadog.trace.api.civisibility.telemetry.tag.Provider @@ -201,8 +200,6 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat InstrumentationBridge.registerBuildEventsHandlerFactory { decorator -> new BuildEventsHandlerImpl<>(buildSystemSessionFactory, new JvmInfoFactoryImpl()) } - - CoveragePerTestBridge.registerCoverageStoreRegistry(coverageStoreFactory) } private static final class MockExecutionSettingsFactory implements ExecutionSettingsFactory { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java index 246ebe2b336..9372b203156 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java @@ -8,7 +8,6 @@ import datadog.trace.api.civisibility.DDTestSuite; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.LibraryCapability; -import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; import datadog.trace.api.civisibility.events.BuildEventsHandler; import datadog.trace.api.civisibility.events.TestEventsHandler; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; @@ -118,7 +117,6 @@ public static void start(Instrumentation inst, SharedCommunicationObjects sco) { TestEventsHandlerFactory testEventsHandlerFactory = new TestEventsHandlerFactory(services, repoServices, coverageServices, executionSettings); InstrumentationBridge.registerTestEventsHandlerFactory(testEventsHandlerFactory); - CoveragePerTestBridge.registerCoverageStoreRegistry(coverageServices.coverageStoreFactory); AgentTracer.TracerAPI tracerAPI = AgentTracer.get(); tracerAPI.addShutdownListener(testEventsHandlerFactory::shutdown); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java index be50bf82d7c..90de26c1be8 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java @@ -31,9 +31,4 @@ public CoverageStore create(@Nullable TestIdentifier testIdentifier) { return delegate.create(testIdentifier); } } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - delegate.setTotalProbeCount(className, totalProbeCount); - } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java index 6de1354e7c7..dfd4017a70a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java @@ -118,10 +118,5 @@ public CoverageStore create(@Nullable TestIdentifier testIdentifier) { private FileProbes createProbes(boolean isTestThread) { return new FileProbes(metrics, isTestThread); } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - // no op - } } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java index ac53ecda24a..47192376abd 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java @@ -31,11 +31,6 @@ public class FileProbes implements CoverageProbes { nonCodeResources = isTestThread ? new HashMap<>() : new ConcurrentHashMap<>(); } - @Override - public void record(Class clazz, long classId, int probeId) { - record(clazz); - } - @Override public void record(Class clazz) { try { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java index 6441eb8ded3..8b6a403faab 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java @@ -5,21 +5,24 @@ public class ExecutionDataAdapter { private final long classId; private final String className; - // Unbounded data structure that only exists within a single test span + // Jacoco's shared probe array for the class, used to back-fill aggregate coverage at report time + private final boolean[] jacocoProbes; + // Per-test probe array that Jacoco's instrumentation writes into while a test is running private final boolean[] probeActivations; - public ExecutionDataAdapter(long classId, String className, int totalProbeCount) { + public ExecutionDataAdapter(long classId, String className, boolean[] jacocoProbes) { this.classId = classId; this.className = className; - this.probeActivations = new boolean[totalProbeCount]; + this.jacocoProbes = jacocoProbes; + this.probeActivations = new boolean[jacocoProbes.length]; } public String getClassName() { return className; } - void record(int probeId) { - probeActivations[probeId] = true; + boolean[] getProbeActivations() { + return probeActivations; } ExecutionDataAdapter merge(ExecutionDataAdapter other) { @@ -29,6 +32,31 @@ ExecutionDataAdapter merge(ExecutionDataAdapter other) { return this; } + /** + * Folds the per-test coverage back into Jacoco's shared probe array. Jacoco's aggregate coverage + * (used for total module/session coverage percentage and report uploads) no longer sees probes + * recorded into the per-test array directly, so they are OR-ed back here at report time. The + * write is monotonic (bits are only ever set), so concurrent back-fills from multiple tests are + * safe. + * + *

The per-test array is allocated when a method of the class is entered (so the probe array + * can be swapped in), which can happen even if no probe ends up firing (e.g. the method throws + * before reaching its first probe). Returning whether any probe was actually covered lets the + * caller skip such classes and avoid emitting empty coverage entries. + * + * @return {@code true} if at least one probe was covered by the test + */ + boolean mergeIntoJacocoProbes() { + boolean covered = false; + for (int i = 0; i < probeActivations.length; i++) { + if (probeActivations[i]) { + jacocoProbes[i] = true; + covered = true; + } + } + return covered; + } + ExecutionData toExecutionData() { return new ExecutionData(classId, className, probeActivations); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java index 647f28ca181..bc7a770ecd1 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java @@ -21,7 +21,6 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import javax.annotation.Nullable; import org.jacoco.core.analysis.Analyzer; @@ -70,6 +69,12 @@ protected TestReport report( Map coveredLinesBySourcePath = new HashMap<>(); for (Map.Entry, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) { ExecutionDataAdapter executionDataAdapter = e.getValue(); + // Back-fill Jacoco's aggregate coverage (total coverage percentage and report uploads). Skip + // classes with no covered probes: the per-test array is allocated on method entry, so a + // method that throws before its first probe fires would otherwise yield an empty entry. + if (!executionDataAdapter.mergeIntoJacocoProbes()) { + continue; + } String className = executionDataAdapter.getClassName(); Class clazz = e.getKey(); @@ -134,8 +139,6 @@ protected TestReport report( public static final class Factory implements CoverageStore.Factory { - private final Map probeCounts = new ConcurrentHashMap<>(); - private final CiVisibilityMetricCollector metrics; private final SourcePathResolver sourcePathResolver; @@ -150,12 +153,7 @@ public CoverageStore create(@Nullable TestIdentifier testIdentifier) { } private LineProbes createProbes(boolean isTestThread) { - return new LineProbes(metrics, probeCounts, isTestThread); - } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - probeCounts.put(className.replace('/', '.'), totalProbeCount); + return new LineProbes(metrics, isTestThread); } } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java index f99c78e2c29..f96c0b5f435 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java @@ -19,7 +19,6 @@ public class LineProbes implements CoverageProbes { private final CiVisibilityMetricCollector metrics; - private final Map probeCounts; private final Map, ExecutionDataAdapter> executionData; private final Map nonCodeResources; @@ -27,10 +26,8 @@ public class LineProbes implements CoverageProbes { private Class lastCoveredClass; private ExecutionDataAdapter lastCoveredExecutionData; - LineProbes( - CiVisibilityMetricCollector metrics, Map probeCounts, boolean isTestThread) { + LineProbes(CiVisibilityMetricCollector metrics, boolean isTestThread) { this.metrics = metrics; - this.probeCounts = probeCounts; executionData = isTestThread ? new IdentityHashMap<>() : new ConcurrentHashMap<>(); nonCodeResources = isTestThread ? new HashMap<>() : new ConcurrentHashMap<>(); } @@ -41,20 +38,21 @@ public void record(Class clazz) { } @Override - public void record(Class clazz, long classId, int probeId) { + public boolean[] resolveProbeArray(Class clazz, long classId, boolean[] jacocoProbes) { try { if (lastCoveredClass != clazz) { - // optimization to avoid map lookup if activating several probes for same class in a row + // optimization to avoid map lookup if resolving the array for the same class in a row lastCoveredExecutionData = executionData.computeIfAbsent( lastCoveredClass = clazz, - k -> new ExecutionDataAdapter(classId, k.getName(), probeCounts.get(k.getName()))); + k -> new ExecutionDataAdapter(classId, k.getName(), jacocoProbes)); } - lastCoveredExecutionData.record(probeId); + return lastCoveredExecutionData.getProbeActivations(); } catch (Exception e) { metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.RECORD); - throw e; + // fall back to Jacoco's shared array so coverage is still recorded + return jacocoProbes; } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java new file mode 100644 index 00000000000..44d5708fcf2 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java @@ -0,0 +1,71 @@ +package datadog.trace.civisibility.coverage.line; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class ExecutionDataAdapterTest { + + @Test + void probeActivationsAreSizedFromJacocoArrayAndStartEmpty() { + boolean[] jacocoProbes = new boolean[4]; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + + boolean[] probeActivations = adapter.getProbeActivations(); + assertEquals(4, probeActivations.length); + assertArrayEquals(new boolean[4], probeActivations); + } + + @Test + void mergeIntoJacocoProbesOrsPerTestBitsBack() { + boolean[] jacocoProbes = new boolean[4]; + // a bit that was already set on the shared array (e.g. covered outside any test) + jacocoProbes[0] = true; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + + // simulate Jacoco's native probes writing into the per-test array + adapter.getProbeActivations()[2] = true; + + adapter.mergeIntoJacocoProbes(); + + // existing shared bit is preserved, per-test bit is folded back, untouched probes stay false + assertArrayEquals(new boolean[] {true, false, true, false}, jacocoProbes); + } + + @Test + void mergeIntoJacocoProbesNeverClearsBits() { + boolean[] jacocoProbes = new boolean[] {true, true, true, true}; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + // per-test array is all false; merging it back must not clear any shared bits + adapter.mergeIntoJacocoProbes(); + assertArrayEquals(new boolean[] {true, true, true, true}, jacocoProbes); + } + + @Test + void mergeCombinesProbeActivationsFromAnotherAdapter() { + boolean[] jacocoProbes = new boolean[4]; + ExecutionDataAdapter a = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + ExecutionDataAdapter b = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + a.getProbeActivations()[1] = true; + b.getProbeActivations()[3] = true; + + ExecutionDataAdapter merged = a.merge(b); + + assertTrue(merged.getProbeActivations()[1]); + assertTrue(merged.getProbeActivations()[3]); + assertFalse(merged.getProbeActivations()[0]); + } + + @Test + void toExecutionDataExposesPerTestProbes() { + boolean[] jacocoProbes = new boolean[3]; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(7L, "com/example/Foo", jacocoProbes); + adapter.getProbeActivations()[1] = true; + + assertEquals(7L, adapter.toExecutionData().getId()); + assertArrayEquals(new boolean[] {false, true, false}, adapter.toExecutionData().getProbes()); + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java new file mode 100644 index 00000000000..b36facd8b6f --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java @@ -0,0 +1,69 @@ +package datadog.trace.civisibility.coverage.line; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.civisibility.coverage.CoverageProbes; +import datadog.trace.api.civisibility.coverage.CoverageStore; +import datadog.trace.api.civisibility.coverage.TestReport; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import datadog.trace.civisibility.source.SourcePathResolver; +import org.junit.jupiter.api.Test; + +class LineCoverageStoreTest { + + private static final class CoveredClass {} + + @Test + void reportFoldsPerTestCoverageBackIntoJacocoSharedArray() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + // source path resolution is irrelevant here: the aggregate back-fill happens regardless + when(sourcePathResolver.getSourcePaths(any())).thenReturn(emptyList()); + + CoverageStore store = new LineCoverageStore.Factory(metrics, sourcePathResolver).create(null); + CoverageProbes probes = store.getProbes(); + + boolean[] jacocoProbes = new boolean[5]; + boolean[] perTest = probes.resolveProbeArray(CoveredClass.class, 42L, jacocoProbes); + // simulate Jacoco's native probes recording coverage into the per-test array + perTest[3] = true; + + // before report, Jacoco's shared (aggregate) array is untouched + assertFalse(jacocoProbes[3]); + + store.report(DDTraceId.ONE, 1L, 1L); + + // after report the per-test coverage is folded back so Jacoco's aggregate stays accurate + assertTrue(jacocoProbes[3]); + } + + @Test + void reportSkipsClassesWhoseProbeArrayWasResolvedButNeverWritten() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + // resolve the source path so the class would be reported if it were not skipped + when(sourcePathResolver.getSourcePaths(any())) + .thenReturn(singletonList("src/test/java/datadog/smoke/CoveredClass.java")); + + CoverageStore store = new LineCoverageStore.Factory(metrics, sourcePathResolver).create(null); + CoverageProbes probes = store.getProbes(); + // a method was entered (probe array resolved) but it threw before any probe fired + probes.resolveProbeArray(CoveredClass.class, 42L, new boolean[5]); + + boolean coverageGathered = store.report(DDTraceId.ONE, 1L, 1L); + + assertFalse( + coverageGathered, "a test that covered no probes must not produce a coverage report"); + TestReport report = store.getReport(); + if (report != null) { + assertTrue(report.getTestReportFileEntries().isEmpty()); + } + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java new file mode 100644 index 00000000000..7ea3bb5a65e --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java @@ -0,0 +1,67 @@ +package datadog.trace.civisibility.coverage.line; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import org.junit.jupiter.api.Test; + +class LineProbesTest { + + private static final class ClassA {} + + private static final class ClassB {} + + private final CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + + @Test + void resolvesAPerTestArraySizedFromJacocoArray() { + LineProbes probes = new LineProbes(metrics, true); + boolean[] jacocoProbes = new boolean[6]; + + boolean[] perTest = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + + assertNotSame(jacocoProbes, perTest, "should not record into Jacoco's shared array"); + assertEquals(jacocoProbes.length, perTest.length); + } + + @Test + void returnsTheSameArrayForRepeatedResolutionsOfTheSameClass() { + LineProbes probes = new LineProbes(metrics, true); + boolean[] jacocoProbes = new boolean[3]; + + boolean[] first = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + boolean[] second = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + + assertSame(first, second); + } + + @Test + void keepsSeparateArraysPerClass() { + LineProbes probes = new LineProbes(metrics, true); + + boolean[] a = probes.resolveProbeArray(ClassA.class, 1L, new boolean[2]); + boolean[] b = probes.resolveProbeArray(ClassB.class, 2L, new boolean[2]); + + assertNotSame(a, b); + assertEquals(2, probes.getExecutionData().size()); + assertTrue(probes.getExecutionData().containsKey(ClassA.class)); + assertTrue(probes.getExecutionData().containsKey(ClassB.class)); + } + + @Test + void perTestWritesDoNotLeakIntoJacocoArrayBeforeReport() { + LineProbes probes = new LineProbes(metrics, true); + boolean[] jacocoProbes = new boolean[4]; + + boolean[] perTest = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + perTest[1] = true; + + // the shared array is only updated at report time via + // ExecutionDataAdapter#mergeIntoJacocoProbes + assertEquals(false, jacocoProbes[1]); + } +} diff --git a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java deleted file mode 100644 index 7960a0ff40e..00000000000 --- a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java +++ /dev/null @@ -1,57 +0,0 @@ -package datadog.trace.instrumentation.jacoco; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.nameEndsWith; -import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.Config; -import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -@AutoService(InstrumenterModule.class) -public class ClassInstrumenterInstrumentation extends InstrumenterModule.CiVisibility - implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public ClassInstrumenterInstrumentation() { - super("jacoco"); - } - - @Override - public boolean isEnabled() { - return super.isEnabled() && Config.get().isCiVisibilityCoverageLinesEnabled(); - } - - @Override - public String hierarchyMarkerType() { - return "org.jacoco.agent.rt.IAgent"; - } - - @Override - public ElementMatcher hierarchyMatcher() { - // The jacoco javaagent jar that is published relocates internal classes to an "obfuscated" - // package name ex. org.jacoco.agent.rt.internal_72ddf3b.core.internal.instr.ClassInstrumenter - return nameStartsWith("org.jacoco.agent.rt.internal") - .and(nameEndsWith(".core.internal.instr.ClassInstrumenter")); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - isMethod().and(named("visitTotalProbeCount")), - getClass().getName() + "$VisitTotalProbeCountAdvice"); - } - - public static class VisitTotalProbeCountAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - static void enter( - @Advice.FieldValue(value = "className") final String className, - @Advice.Argument(0) int count) { - CoveragePerTestBridge.setTotalProbeCount(className, count); - } - } -} diff --git a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java index 8b80d00ce30..53b394ab997 100644 --- a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java +++ b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java @@ -17,6 +17,7 @@ public class MethodVisitorWrapper { private static final MethodHandle visitMethodInsnHandle; private static final MethodHandle visitInsnHandle; private static final MethodHandle visitIntInsnHandle; + private static final MethodHandle visitVarInsnHandle; private static final MethodHandle visitLdcInsnHandle; private static final MethodHandle getTypeHandle; @@ -43,6 +44,8 @@ public class MethodVisitorWrapper { visitInsnHandle = accessMethod(lookup, shadedMethodVisitorClass, "visitInsn", int.class); visitIntInsnHandle = accessMethod(lookup, shadedMethodVisitorClass, "visitIntInsn", int.class, int.class); + visitVarInsnHandle = + accessMethod(lookup, shadedMethodVisitorClass, "visitVarInsn", int.class, int.class); visitLdcInsnHandle = accessMethod(lookup, shadedMethodVisitorClass, "visitLdcInsn", Object.class); @@ -97,6 +100,10 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, visitMethodInsnHandle.invoke(mv, opcode, owner, name, desc, itf); } + public void visitVarInsn(int opcode, int var) throws Throwable { + visitVarInsnHandle.invoke(mv, opcode, var); + } + public void visitLdcInsn(Object cst) throws Throwable { visitLdcInsnHandle.invoke(mv, cst); } diff --git a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java index 695b659d6e1..ad7929de302 100644 --- a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java +++ b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java @@ -112,29 +112,17 @@ public ElementMatcher hierarchyMatcher() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isMethod().and(named("visitMaxs")).and(takesArguments(2)).and(takesArgument(0, int.class)), - getClass().getName() + "$VisitMaxsAdvice"); - transformer.applyAdvice( - isMethod() - .and(named("insertProbe")) - .and(takesArguments(1)) - .and(takesArgument(0, int.class)), - getClass().getName() + "$InsertProbeAdvice"); - } - - public static class VisitMaxsAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - static void enter(@Advice.Argument(value = 0, readOnly = false) int maxStack) { - maxStack = maxStack + 2; - } + isMethod().and(named("visitCode")).and(takesArguments(0)), + getClass().getName() + "$VisitCodeAdvice"); } - public static class InsertProbeAdvice { - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static class VisitCodeAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) static void exit( @Advice.FieldValue(value = "mv") final Object mv, @Advice.FieldValue(value = "arrayStrategy") final Object arrayStrategy, - @Advice.Argument(0) final int id) + @Advice.FieldValue(value = "variable") final int variable, + @Advice.FieldValue(value = "accessorStackSize", readOnly = false) int accessorStackSize) throws Throwable { Field classNameField = arrayStrategy.getClass().getDeclaredField("className"); classNameField.setAccessible(true); @@ -167,16 +155,24 @@ static void exit( MethodVisitorWrapper methodVisitor = MethodVisitorWrapper.wrap(mv); + // Jacoco's storeInstance() has just stored the class' shared probe array into local variable + // `variable`. Swap it for the per-test array so Jacoco's own probe writes + // (probes[id] = true) record per-test coverage with no per-probe overhead. When no test is + // active the bridge returns the shared array unchanged, preserving Jacoco's aggregate. + methodVisitor.visitVarInsn(Opcodes.ALOAD, variable); methodVisitor.pushClass(className); methodVisitor.visitLdcInsn(classId); - methodVisitor.push(id); - methodVisitor.visitMethodInsn( Opcodes.INVOKESTATIC, "datadog/trace/api/civisibility/coverage/CoveragePerTestBridge", - "recordCoverage", - "(Ljava/lang/Class;JI)V", + "resolveProbeArray", + "([ZLjava/lang/Class;J)[Z", false); + methodVisitor.visitVarInsn(Opcodes.ASTORE, variable); + + // the swap leaves 4 slots on the stack (boolean[] + Class + long); Jacoco sizes the method's + // max stack as max(maxStack + 3, accessorStackSize) in visitMaxs + accessorStackSize = Math.max(accessorStackSize, 4); } } } diff --git a/internal-api/build.gradle.kts b/internal-api/build.gradle.kts index 6e99abe89d2..cb6e5f4f0e2 100644 --- a/internal-api/build.gradle.kts +++ b/internal-api/build.gradle.kts @@ -127,6 +127,7 @@ extra["excludedClassesCoverage"] = listOf( "datadog.trace.api.civisibility.coverage.CoveragePerTestBridge", "datadog.trace.api.civisibility.coverage.CoveragePerTestBridge.TotalProbeCount", "datadog.trace.api.civisibility.coverage.CoveragePercentageBridge", + "datadog.trace.api.civisibility.coverage.CoverageProbes", "datadog.trace.api.civisibility.coverage.NoOpCoverageStore", "datadog.trace.api.civisibility.coverage.NoOpCoverageStore.Factory", "datadog.trace.api.civisibility.coverage.NoOpProbes", diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java index c8f52293eec..744b50b6ac7 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java @@ -2,73 +2,14 @@ import datadog.trace.api.civisibility.InstrumentationTestBridge; import datadog.trace.api.civisibility.domain.TestContext; -import java.util.ArrayDeque; -import java.util.Queue; -import javax.annotation.Nonnull; -import javax.annotation.concurrent.GuardedBy; public abstract class CoveragePerTestBridge { private static final ThreadLocal COVERAGE_PROBES = new ThreadLocal<>(); - private static volatile CoverageStore.Registry COVERAGE_STORE_REGISTRY; - private static final Object COVERAGE_STORE_REGISTRY_LOCK = new Object(); - - @GuardedBy("COVERAGE_STORE_REGISTRY_LOCK") - private static final Queue DEFERRED_PROBE_COUNTS = new ArrayDeque<>(); - - public static void registerCoverageStoreRegistry( - @Nonnull CoverageStore.Registry coverageStoreRegistry) { - synchronized (COVERAGE_STORE_REGISTRY_LOCK) { - while (!DEFERRED_PROBE_COUNTS.isEmpty()) { - TotalProbeCount c = DEFERRED_PROBE_COUNTS.poll(); - coverageStoreRegistry.setTotalProbeCount(c.className, c.count); - } - COVERAGE_STORE_REGISTRY = coverageStoreRegistry; - } - } - - /** - * {@link #COVERAGE_STORE_REGISTRY} is set when CI Visibility is initialized. It is possible, that - * core/internal JDK classes are loaded and transformed by Jacoco before this happens. As the - * result this method may be called when {@link #COVERAGE_STORE_REGISTRY} is still {@code null}. - * - *

While instrumenting core/internal JDK classes with Jacoco makes little sense, we do not - * always have the control over the users' Jacoco {@code includes} setting, therefore we have to - * account for this case and support it. - * - *

If this method finds {@link #COVERAGE_STORE_REGISTRY} to be {@code null}, the probe counts - * are saved in {@link #DEFERRED_PROBE_COUNTS} to be processed when {@link - * #COVERAGE_STORE_REGISTRY} is set. - */ - public static void setTotalProbeCount(String className, int totalProbeCount) { - if (COVERAGE_STORE_REGISTRY != null) { - COVERAGE_STORE_REGISTRY.setTotalProbeCount(className, totalProbeCount); - return; - } - - synchronized (COVERAGE_STORE_REGISTRY_LOCK) { - if (COVERAGE_STORE_REGISTRY != null) { - COVERAGE_STORE_REGISTRY.setTotalProbeCount(className, totalProbeCount); - } else { - DEFERRED_PROBE_COUNTS.offer(new TotalProbeCount(className, totalProbeCount)); - } - } - } - - private static final class TotalProbeCount { - private final String className; - private final int count; - - private TotalProbeCount(String className, int count) { - this.className = className; - this.count = count; - } - } - - /* This method is referenced by name in bytecode added in jacoco instrumentation module (see datadog.trace.instrumentation.jacoco.ProbeInserterInstrumentation.InsertProbeAdvice) */ - public static void recordCoverage(Class clazz, long classId, int probeId) { - getCurrentCoverageProbes().record(clazz, classId, probeId); + /* This method is referenced by name in bytecode added in jacoco instrumentation module (see datadog.trace.instrumentation.jacoco.ProbeInserterInstrumentation.VisitCodeAdvice) */ + public static boolean[] resolveProbeArray(boolean[] jacocoProbes, Class clazz, long classId) { + return getCurrentCoverageProbes().resolveProbeArray(clazz, classId, jacocoProbes); } /* This method is referenced by name in bytecode added by coverage probes (see datadog.trace.civisibility.coverage.instrumentation.CoverageUtils#insertCoverageProbe) */ diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java index 2bdb5ca2371..54535de4bc6 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java @@ -3,7 +3,20 @@ public interface CoverageProbes { void record(Class clazz); - void record(Class clazz, long classId, int probeId); + /** + * Resolves the probe array that Jacoco's instrumentation writes into at runtime. Called once per + * instrumented method invocation, allowing per-test coverage to be captured by swapping Jacoco's + * shared probe array for a test-scoped one. The default returns {@code jacocoProbes} unchanged so + * that, when no per-test store is active, Jacoco's own aggregate coverage keeps working. + * + * @param clazz the class being executed + * @param classId Jacoco's class identifier + * @param jacocoProbes Jacoco's shared probe array for the class + * @return the probe array to record coverage into + */ + default boolean[] resolveProbeArray(Class clazz, long classId, boolean[] jacocoProbes) { + return jacocoProbes; + } void recordNonCodeResource(String absolutePath); } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java index a779c699281..cc42d28aa98 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java @@ -13,11 +13,7 @@ public interface CoverageStore extends TestReportHolder { */ boolean report(DDTraceId testSessionId, Long testSuiteId, long testSpanId); - interface Factory extends Registry { + interface Factory { CoverageStore create(@Nullable TestIdentifier testIdentifier); } - - interface Registry { - void setTotalProbeCount(String className, int totalProbeCount); - } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java index c2b677f416f..7c709eaff1e 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java @@ -31,10 +31,5 @@ public static final class Factory implements CoverageStore.Factory { public CoverageStore create(@Nullable TestIdentifier testIdentifier) { return INSTANCE; } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - // no op - } } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java index cdc2142e398..dfd55855250 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java @@ -8,9 +8,6 @@ private NoOpProbes() {} @Override public void record(Class clazz) {} - @Override - public void record(Class clazz, long classId, int probeId) {} - @Override public void recordNonCodeResource(String absolutePath) {} } From 826244aaef24bfb0747f6890e176a1f13c9665c3 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 13 Jul 2026 15:57:53 +0200 Subject: [PATCH 2/2] Memoize per-test class analysis in a module-wide cache Jacoco's Analyzer re-parsed each covered class once per test; cache the covered lines per (class id, probe set) so a class covered identically by many tests is analyzed only once. Co-Authored-By: Claude Opus 4.8 --- .../coverage/line/ExecutionDataAdapter.java | 4 + .../coverage/line/LineCoverageStore.java | 109 ++++++++++++++---- .../coverage/line/LineCoverageStoreTest.java | 17 +++ 3 files changed, 110 insertions(+), 20 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java index 8b6a403faab..f35d8cfbe25 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapter.java @@ -21,6 +21,10 @@ public String getClassName() { return className; } + long getClassId() { + return classId; + } + boolean[] getProbeActivations() { return probeActivations; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java index bc7a770ecd1..6eeebd083c9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java @@ -14,6 +14,7 @@ import datadog.trace.civisibility.source.Utils; import java.io.InputStream; import java.util.ArrayList; +import java.util.Arrays; import java.util.BitSet; import java.util.Collection; import java.util.HashMap; @@ -21,6 +22,7 @@ import java.util.IdentityHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import javax.annotation.Nullable; import org.jacoco.core.analysis.Analyzer; @@ -36,16 +38,27 @@ public class LineCoverageStore extends ConcurrentCoverageStore { private static final Logger log = LoggerFactory.getLogger(LineCoverageStore.class); + /** + * Upper bound on the number of cached class analyses. Coverage stays correct beyond it (analysis + * just isn't cached), this only guards memory for pathologically large suites. + */ + private static final int MAX_ANALYSIS_CACHE_ENTRIES = 50_000; + private final CiVisibilityMetricCollector metrics; private final SourcePathResolver sourcePathResolver; + // Module-wide cache: (class id + probe set) -> covered lines, shared across tests so a class + // covered identically by many tests is parsed by Jacoco's Analyzer only once. + private final Map analysisCache; private LineCoverageStore( Function probesFactory, CiVisibilityMetricCollector metrics, - SourcePathResolver sourcePathResolver) { + SourcePathResolver sourcePathResolver, + Map analysisCache) { super(probesFactory); this.metrics = metrics; this.sourcePathResolver = sourcePathResolver; + this.analysisCache = analysisCache; } @Nullable @@ -88,24 +101,9 @@ protected TestReport report( } String sourcePath = sourcePaths.iterator().next(); - try (InputStream is = Utils.getClassStream(clazz)) { - BitSet coveredLines = - coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet()); - ExecutionDataStore store = new ExecutionDataStore(); - store.put(executionDataAdapter.toExecutionData()); - - // TODO optimize this part to avoid parsing - // the same class multiple times for different test cases - Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines)); - analyzer.analyzeClass(is, null); - - } catch (Exception exception) { - log.debug( - "Skipping coverage reporting for {} ({}) because of error", - className, - sourcePath, - exception); - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); + BitSet coveredLines = analyzeClass(clazz, executionDataAdapter); + if (coveredLines != null) { + coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet()).or(coveredLines); } } @@ -137,10 +135,81 @@ protected TestReport report( return report; } + /** + * Resolves the covered lines for a class given a test's probe activations. Parsing the class with + * Jacoco's {@link Analyzer} is the dominant cost of reporting, and the result depends only on the + * class bytecode and the probe set, so it is memoized: the same class covered identically by + * different tests is parsed once. + * + * @return the covered lines, or {@code null} if the class could not be analyzed + */ + @Nullable + private BitSet analyzeClass(Class clazz, ExecutionDataAdapter executionDataAdapter) { + AnalysisCacheKey key = + new AnalysisCacheKey( + executionDataAdapter.getClassId(), executionDataAdapter.getProbeActivations()); + BitSet cached = analysisCache.get(key); + if (cached != null) { + return cached; + } + + try (InputStream is = Utils.getClassStream(clazz)) { + BitSet coveredLines = new BitSet(); + ExecutionDataStore store = new ExecutionDataStore(); + store.put(executionDataAdapter.toExecutionData()); + Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines)); + analyzer.analyzeClass(is, null); + + if (analysisCache.size() < MAX_ANALYSIS_CACHE_ENTRIES) { + analysisCache.putIfAbsent(key, coveredLines); + } + return coveredLines; + + } catch (Exception exception) { + log.debug( + "Skipping coverage reporting for {} because of error", + executionDataAdapter.getClassName(), + exception); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); + return null; + } + } + + /** Cache key identifying a class (by Jacoco class id) covered by a specific set of probes. */ + static final class AnalysisCacheKey { + private final long classId; + private final boolean[] probes; + private final int hash; + + AnalysisCacheKey(long classId, boolean[] probes) { + this.classId = classId; + this.probes = probes; + this.hash = 31 * Long.hashCode(classId) + Arrays.hashCode(probes); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof AnalysisCacheKey)) { + return false; + } + AnalysisCacheKey other = (AnalysisCacheKey) o; + return classId == other.classId && hash == other.hash && Arrays.equals(probes, other.probes); + } + + @Override + public int hashCode() { + return hash; + } + } + public static final class Factory implements CoverageStore.Factory { private final CiVisibilityMetricCollector metrics; private final SourcePathResolver sourcePathResolver; + private final Map analysisCache = new ConcurrentHashMap<>(); public Factory(CiVisibilityMetricCollector metrics, SourcePathResolver sourcePathResolver) { this.metrics = metrics; @@ -149,7 +218,7 @@ public Factory(CiVisibilityMetricCollector metrics, SourcePathResolver sourcePat @Override public CoverageStore create(@Nullable TestIdentifier testIdentifier) { - return new LineCoverageStore(this::createProbes, metrics, sourcePathResolver); + return new LineCoverageStore(this::createProbes, metrics, sourcePathResolver, analysisCache); } private LineProbes createProbes(boolean isTestThread) { diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java index b36facd8b6f..8928e7587bb 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java @@ -2,7 +2,9 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +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.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -66,4 +68,19 @@ void reportSkipsClassesWhoseProbeArrayWasResolvedButNeverWritten() { assertTrue(report.getTestReportFileEntries().isEmpty()); } } + + @Test + void analysisCacheKeyDistinguishesClassesAndProbeSets() { + boolean[] probes = {true, false, true}; + boolean[] sameProbes = {true, false, true}; + boolean[] otherProbes = {true, true, true}; + + LineCoverageStore.AnalysisCacheKey key = new LineCoverageStore.AnalysisCacheKey(1L, probes); + // identical class id + probe contents must collide so the analysis is reused + assertEquals(key, new LineCoverageStore.AnalysisCacheKey(1L, sameProbes)); + assertEquals(key.hashCode(), new LineCoverageStore.AnalysisCacheKey(1L, sameProbes).hashCode()); + // a different class or a different probe set must NOT hit the same cache entry + assertNotEquals(key, new LineCoverageStore.AnalysisCacheKey(2L, probes)); + assertNotEquals(key, new LineCoverageStore.AnalysisCacheKey(1L, otherProbes)); + } }