Collect worker metrics by default.
Flag `experimental_collect_worker_data_in_profiler` moved to the graveyard.
PiperOrigin-RevId: 628042522
Change-Id: I7e680a1b55c90aa227263bec12a23673e486b219
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
index 25b5ee5..03096de 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java
@@ -315,6 +315,14 @@
help = "No-op",
oldName = "incompatible_build_transitive_python_runfiles")
public boolean buildTransitiveRunfilesTrees;
+
+ @Option(
+ name = "experimental_collect_worker_data_in_profiler",
+ defaultValue = "true",
+ documentationCategory = OptionDocumentationCategory.LOGGING,
+ effectTags = {OptionEffectTag.NO_OP},
+ help = "No-op.")
+ public boolean collectWorkerDataInProfiler;
}
/** This is where deprecated Bazel-specific options only used by the build command go to die. */
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 e684f2f..f96ca55 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
@@ -53,7 +53,6 @@
private static final Duration LOCAL_RESOURCES_COLLECT_SLEEP_INTERVAL = Duration.ofMillis(200);
private final BugReporter bugReporter;
- private final boolean collectWorkerDataInProfiler;
private final boolean collectLoadAverage;
private final boolean collectSystemNetworkUsage;
private final boolean collectResourceManagerEstimation;
@@ -82,13 +81,11 @@
BugReporter bugReporter,
WorkerProcessMetricsCollector workerProcessMetricsCollector,
ResourceEstimator resourceEstimator,
- boolean collectWorkerDataInProfiler,
boolean collectLoadAverage,
boolean collectSystemNetworkUsage,
boolean collectResourceManagerEstimation,
boolean collectPressureStallIndicators) {
this.bugReporter = checkNotNull(bugReporter);
- this.collectWorkerDataInProfiler = collectWorkerDataInProfiler;
this.workerProcessMetricsCollector = workerProcessMetricsCollector;
this.collectLoadAverage = collectLoadAverage;
this.collectSystemNetworkUsage = collectSystemNetworkUsage;
@@ -125,11 +122,9 @@
ProfilerTask.LOCAL_CPU_USAGE,
ProfilerTask.LOCAL_MEMORY_USAGE,
ProfilerTask.SYSTEM_CPU_USAGE,
- ProfilerTask.SYSTEM_MEMORY_USAGE));
+ ProfilerTask.SYSTEM_MEMORY_USAGE,
+ ProfilerTask.WORKERS_MEMORY_USAGE));
- if (collectWorkerDataInProfiler) {
- enabledCounters.add(ProfilerTask.WORKERS_MEMORY_USAGE);
- }
if (collectLoadAverage) {
enabledCounters.add(ProfilerTask.SYSTEM_LOAD_AVERAGE);
}
@@ -220,8 +215,8 @@
}
int workerMemoryUsageMb = 0;
- if (collectWorkerDataInProfiler) {
- try (SilentCloseable c = Profiler.instance().profile("Worker metrics collection")) {
+ try (SilentCloseable c = Profiler.instance().profile("Worker metrics collection")) {
+ if (OS.getCurrent() == OS.LINUX || OS.getCurrent() == OS.DARWIN) {
workerMemoryUsageMb =
workerProcessMetricsCollector.getLiveWorkerProcessMetrics().stream()
.mapToInt(WorkerProcessMetrics::getUsedMemoryInKb)
@@ -278,8 +273,13 @@
previousElapsed,
nextElapsed,
(double) systemMemoryUsageMb);
- addRange(
- ProfilerTask.WORKERS_MEMORY_USAGE, previousElapsed, nextElapsed, workerMemoryUsageMb);
+ if (workerMemoryUsageMb > 0) {
+ addRange(
+ ProfilerTask.WORKERS_MEMORY_USAGE,
+ previousElapsed,
+ nextElapsed,
+ workerMemoryUsageMb);
+ }
if (loadAverage > 0) {
addRange(ProfilerTask.SYSTEM_LOAD_AVERAGE, previousElapsed, nextElapsed, loadAverage);
}
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 298e945..88e153e 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
@@ -401,7 +401,6 @@
bugReporter,
workerProcessMetricsCollector,
env.getLocalResourceManager(),
- options.collectWorkerDataInProfiler,
options.collectLoadAverageInProfiler,
options.collectSystemNetworkUsage,
options.collectResourceEstimation,
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
index 9657df3..3ebc05c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java
@@ -296,14 +296,6 @@
public boolean recordFullProfilerData;
@Option(
- name = "experimental_collect_worker_data_in_profiler",
- defaultValue = "false",
- documentationCategory = OptionDocumentationCategory.LOGGING,
- effectTags = {OptionEffectTag.BAZEL_MONITORING},
- help = "If enabled, the profiler collects worker's aggregated resource data.")
- public boolean collectWorkerDataInProfiler;
-
- @Option(
name = "experimental_collect_load_average_in_profiler",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.LOGGING,
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java
index be66585..faf062e 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerProcessMetricsCollector.java
@@ -23,6 +23,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.WorkerMetrics;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.WorkerMetrics.WorkerStatus;
+import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.metrics.CgroupsInfoCollector;
import com.google.devtools.build.lib.metrics.PsInfoCollector;
@@ -46,7 +47,7 @@
private final PsInfoCollector psInfoCollector;
private final CgroupsInfoCollector cgroupsInfoCollector;
- private Clock clock;
+ private Clock clock = BlazeClock.instance();
/**
* Mapping of worker process ids to their process metrics. This contains all workers that have
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 5739f4b..730c799 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
@@ -413,7 +413,6 @@
runtime.getBugReporter(),
WorkerProcessMetricsCollector.instance(),
env.getLocalResourceManager(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
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 6b56d9c..d12466b 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
@@ -115,7 +115,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -141,7 +140,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -252,7 +250,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -336,7 +333,6 @@
BugReporter.defaultInstance(),
workerProcessMetricsCollector,
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ true,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -373,7 +369,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -504,7 +499,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -623,7 +617,8 @@
profiler.stop();
JsonProfile jsonProfile = new JsonProfile(new ByteArrayInputStream(buffer.toByteArray()));
- List<TraceEvent> filteredEvents = removeCounterEvents(jsonProfile.getTraceEvents());
+ ImmutableList<TraceEvent> filteredEvents =
+ removeWorkerMetricsEvents(removeCounterEvents(jsonProfile.getTraceEvents()));
assertThat(filteredEvents)
.hasSize(
4 /* thread names */
@@ -648,6 +643,20 @@
return events.stream().filter(e -> !"C".equals(e.type())).collect(toImmutableList());
}
+ private static ImmutableList<TraceEvent> removeWorkerMetricsEvents(List<TraceEvent> events) {
+ ImmutableList<TraceEvent> workerMetricsEvents =
+ events.stream()
+ .filter(e -> e.name().equals("Worker metrics collection"))
+ .collect(toImmutableList());
+ if (workerMetricsEvents.isEmpty()) {
+ return ImmutableList.copyOf(events);
+ }
+ long workerMetricsThreadId = workerMetricsEvents.get(0).threadId();
+ return events.stream()
+ .filter(e -> e.threadId() != workerMetricsThreadId)
+ .collect(toImmutableList());
+ }
+
/**
* Extracts all events for a given phase.
*
@@ -708,7 +717,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -775,7 +783,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -812,7 +819,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -844,7 +850,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -884,7 +889,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
@@ -923,7 +927,6 @@
BugReporter.defaultInstance(),
WorkerProcessMetricsCollector.instance(),
ResourceManager.instance(),
- /* collectWorkerDataInProfiler= */ false,
/* collectLoadAverage= */ false,
/* collectSystemNetworkUsage= */ false,
/* collectResourceManagerEstimation= */ false,
