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;
