Explicitly pass BugReporter through to the profiler etc This previously wasn't possible because it would create a cycle in the dependency graph, hence the Consumer indirection. That cycle is gone. PiperOrigin-RevId: 393862621
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/BUILD b/src/main/java/com/google/devtools/build/lib/profiler/BUILD index e764016..9c77784 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/BUILD +++ b/src/main/java/com/google/devtools/build/lib/profiler/BUILD
@@ -25,6 +25,7 @@ "TimeSeries.java", ], deps = [ + "//src/main/java/com/google/devtools/build/lib/bugreport", "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/collect:extrema", "//src/main/java/com/google/devtools/build/lib/concurrent",
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/CollectLocalResourceUsage.java b/src/main/java/com/google/devtools/build/lib/profiler/CollectLocalResourceUsage.java index 74cb7cc..6fdf14c 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/CollectLocalResourceUsage.java +++ b/src/main/java/com/google/devtools/build/lib/profiler/CollectLocalResourceUsage.java
@@ -17,13 +17,13 @@ import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; +import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.errorprone.annotations.concurrent.GuardedBy; import com.sun.management.OperatingSystemMXBean; import java.lang.management.ManagementFactory; import java.lang.management.MemoryMXBean; import java.time.Duration; import java.util.concurrent.TimeUnit; -import java.util.function.Consumer; /** Thread to collect local resource usage data and log into JSON profile. */ public class CollectLocalResourceUsage extends Thread { @@ -31,7 +31,7 @@ private static final Duration BUCKET_DURATION = Duration.ofSeconds(1); private static final long LOCAL_CPU_SLEEP_MILLIS = 200; - private final Consumer<Exception> bugReporter; + private final BugReporter bugReporter; private volatile boolean stopLocalUsageCollection; private volatile boolean profilingStarted; @@ -44,7 +44,7 @@ private Stopwatch stopwatch; - CollectLocalResourceUsage(Consumer<Exception> bugReporter) { + CollectLocalResourceUsage(BugReporter bugReporter) { this.bugReporter = checkNotNull(bugReporter); } @@ -81,7 +81,7 @@ + memoryBean.getNonHeapMemoryUsage().getUsed(); } catch (IllegalArgumentException e) { // The JVM may report committed > max. See b/180619163. - bugReporter.accept(e); + bugReporter.sendBugReport(e); memoryUsage = -1; }
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java b/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java index e52a593..7ab40ff 100644 --- a/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java +++ b/src/main/java/com/google/devtools/build/lib/profiler/Profiler.java
@@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.collect.Extrema; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; @@ -48,7 +49,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.zip.GZIPOutputStream; @@ -371,7 +371,7 @@ boolean includePrimaryOutput, boolean includeTargetLabel, boolean collectTaskHistograms, - Consumer<Exception> bugReporter) + BugReporter bugReporter) throws IOException { Preconditions.checkState(!isActive(), "Profiler already active"); initHistograms();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 56a27e7..04e08cc 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -405,7 +405,7 @@ options.includePrimaryOutput, options.profileIncludeTargetLabel, options.alwaysProfileSlowOperations, - bugReporter::sendBugReport); + bugReporter); // Instead of logEvent() we're calling the low level function to pass the timings we took in // the launcher. We're setting the INIT phase marker so that it follows immediately the // LAUNCH phase.
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java index 5fa6138..6f5a54b 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
@@ -314,7 +314,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - runtime.getBugReporter()::sendBugReport); + runtime.getBugReporter()); StoredEventHandler storedEventHandler = new StoredEventHandler(); reporter.addHandler(storedEventHandler);
diff --git a/src/test/java/com/google/devtools/build/lib/profiler/ProfilerTest.java b/src/test/java/com/google/devtools/build/lib/profiler/ProfilerTest.java index 414d232..5ccee2e 100644 --- a/src/test/java/com/google/devtools/build/lib/profiler/ProfilerTest.java +++ b/src/test/java/com/google/devtools/build/lib/profiler/ProfilerTest.java
@@ -22,7 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.bugreport.BugReport; +import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.clock.JavaClock; @@ -105,7 +105,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); return buffer; } @@ -124,7 +124,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); } @Test @@ -231,7 +231,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); try (SilentCloseable c = profiler.profile(ProfilerTask.ACTION, "action task")) { // Next task takes less than 10 ms but should be recorded anyway. long before = clock.nanoTime(); @@ -281,7 +281,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); profiler.logSimpleTask(10000, 20000, ProfilerTask.VFS_STAT, "stat"); // Unlike the VFS_STAT event above, the remote execution event will not be recorded since we // don't record the slowest remote exec events (see ProfilerTask.java). @@ -402,7 +402,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); profiler.logSimpleTask(10000, 20000, ProfilerTask.VFS_STAT, "stat"); assertThat(ProfilerTask.VFS_STAT.collectsSlowestInstances()).isTrue(); @@ -594,7 +594,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); profiler.logSimpleTask(badClock.nanoTime(), ProfilerTask.INFO, "some task"); profiler.stop(); } @@ -653,7 +653,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); profiler.logSimpleTaskDuration( Profiler.nanoTimeMaybe(), Duration.ofSeconds(10), ProfilerTask.INFO, "foo"); IOException expected = assertThrows(IOException.class, profiler::stop); @@ -682,7 +682,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); profiler.logSimpleTaskDuration( Profiler.nanoTimeMaybe(), Duration.ofSeconds(10), ProfilerTask.INFO, "foo"); IOException expected = assertThrows(IOException.class, profiler::stop); @@ -707,7 +707,7 @@ /*includePrimaryOutput=*/ true, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); try (SilentCloseable c = profiler.profileAction(ProfilerTask.ACTION, "test", "foo.out", "")) { profiler.logEvent(ProfilerTask.PHASE, "event1"); } @@ -740,7 +740,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ true, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); try (SilentCloseable c = profiler.profileAction(ProfilerTask.ACTION, "test", "foo.out", "//foo:bar")) { profiler.logEvent(ProfilerTask.PHASE, "event1"); @@ -772,7 +772,7 @@ /*includePrimaryOutput=*/ false, /*includeTargetLabel=*/ false, /*collectTaskHistograms=*/ true, - BugReport::sendBugReport); + BugReporter.defaultInstance()); long curTime = Profiler.nanoTimeMaybe(); for (int i = 0; i < 100_000; i++) { Duration duration;