Change --memory_profile_stable_heap_parameters to accept more than one GC specification
Currently memory_profile_stable_heap_parameters expects 2 ints and runs that
many GCs with pauses between them 2nd param.
This CL doesn't change that, but allows any arbitrary number of pairs to be
provided that will run the same logic for each pair. This allows experimenting
with forcing longer pauses on that thread before doing the quick GCs that allow
for cleaner memory measurement.
PiperOrigin-RevId: 485646588
Change-Id: Iff4f17cdaae409854f99397b4271bb5f87c4c404
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 a4e809d..17f1f8f 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/profiler/BUILD
@@ -33,6 +33,7 @@
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/unix:procmeminfo_parser",
"//src/main/java/com/google/devtools/build/lib/util:os",
+ "//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/java/com/google/devtools/build/lib/worker:worker_metric",
"//src/main/java/com/google/devtools/common/options",
"//third_party:auto_value",
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/MemoryProfiler.java b/src/main/java/com/google/devtools/build/lib/profiler/MemoryProfiler.java
index 48c05b8..4cab893 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/MemoryProfiler.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/MemoryProfiler.java
@@ -18,6 +18,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Splitter;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.common.options.OptionsParsingException;
import java.io.OutputStream;
import java.io.PrintStream;
@@ -25,7 +26,8 @@
import java.lang.management.MemoryMXBean;
import java.lang.management.MemoryUsage;
import java.time.Duration;
-import java.util.Iterator;
+import java.util.ArrayList;
+import java.util.List;
import java.util.NoSuchElementException;
import javax.annotation.Nullable;
@@ -111,14 +113,29 @@
bean.gc();
MemoryUsage minHeapUsed = bean.getHeapMemoryUsage();
MemoryUsage minNonHeapUsed = bean.getNonHeapMemoryUsage();
+
if (nextPhase == ProfilePhase.FINISH && memoryProfileStableHeapParameters != null) {
- for (int i = 1; i < memoryProfileStableHeapParameters.numTimesToDoGc; i++) {
- sleeper.sleep(memoryProfileStableHeapParameters.timeToSleepBetweenGcs);
- bean.gc();
- MemoryUsage currentHeapUsed = bean.getHeapMemoryUsage();
- if (currentHeapUsed.getUsed() < minHeapUsed.getUsed()) {
- minHeapUsed = currentHeapUsed;
- minNonHeapUsed = bean.getNonHeapMemoryUsage();
+ for (int j = 0; j < memoryProfileStableHeapParameters.gcSpecs.size(); j++) {
+ Pair<Integer, Duration> spec = memoryProfileStableHeapParameters.gcSpecs.get(j);
+
+ int numTimesToDoGc = spec.first;
+ Duration timeToSleepBetweenGcs = spec.second;
+
+ for (int i = 0; i < numTimesToDoGc; i++) {
+ // We want to skip the first cycle for the first spec, since we ran a
+ // GC at the top of this function, but all the rest should get their
+ // proper runs
+ if (j == 0 && i == 0) {
+ continue;
+ }
+
+ sleeper.sleep(timeToSleepBetweenGcs);
+ bean.gc();
+ MemoryUsage currentHeapUsed = bean.getHeapMemoryUsage();
+ if (currentHeapUsed.getUsed() < minHeapUsed.getUsed()) {
+ minHeapUsed = currentHeapUsed;
+ minNonHeapUsed = bean.getNonHeapMemoryUsage();
+ }
}
}
}
@@ -130,12 +147,10 @@
* build.
*/
public static class MemoryProfileStableHeapParameters {
- private final int numTimesToDoGc;
- private final Duration timeToSleepBetweenGcs;
+ private final ArrayList<Pair<Integer, Duration>> gcSpecs;
- private MemoryProfileStableHeapParameters(int numTimesToDoGc, Duration timeToSleepBetweenGcs) {
- this.numTimesToDoGc = numTimesToDoGc;
- this.timeToSleepBetweenGcs = timeToSleepBetweenGcs;
+ private MemoryProfileStableHeapParameters(ArrayList<Pair<Integer, Duration>> gcSpecs) {
+ this.gcSpecs = gcSpecs;
}
/** Converter for {@code MemoryProfileStableHeapParameters} option. */
@@ -147,40 +162,48 @@
@Override
public MemoryProfileStableHeapParameters convert(String input)
throws OptionsParsingException {
- Iterator<String> values = SPLITTER.split(input).iterator();
+ List<String> values = SPLITTER.splitToList(input);
+
+ if (values.size() % 2 != 0) {
+ throw new OptionsParsingException(
+ "Expected even number of comma-separated integer values");
+ }
+
+ ArrayList<Pair<Integer, Duration>> gcSpecs = new ArrayList<>(values.size() / 2);
+
try {
- int numTimesToDoGc = Integer.parseInt(values.next());
- int numSecondsToSleepBetweenGcs = Integer.parseInt(values.next());
- if (values.hasNext()) {
- throw new OptionsParsingException("Expected exactly 2 comma-separated integer values");
+ for (int i = 0; i < values.size(); i += 2) {
+ int numTimesToDoGc = Integer.parseInt(values.get(i));
+ int numSecondsToSleepBetweenGcs = Integer.parseInt(values.get(i + 1));
+
+ if (numTimesToDoGc <= 0) {
+ throw new OptionsParsingException("Number of times to GC must be positive");
+ }
+ if (numSecondsToSleepBetweenGcs < 0) {
+ throw new OptionsParsingException(
+ "Number of seconds to sleep between GC's must be non-negative");
+ }
+ gcSpecs.add(Pair.of(numTimesToDoGc, Duration.ofSeconds(numSecondsToSleepBetweenGcs)));
}
- if (numTimesToDoGc <= 0) {
- throw new OptionsParsingException("Number of times to GC must be positive");
- }
- if (numSecondsToSleepBetweenGcs < 0) {
- throw new OptionsParsingException(
- "Number of seconds to sleep between GC's must be non-negative");
- }
- return new MemoryProfileStableHeapParameters(
- numTimesToDoGc, Duration.ofSeconds(numSecondsToSleepBetweenGcs));
+
+ return new MemoryProfileStableHeapParameters(gcSpecs);
} catch (NumberFormatException | NoSuchElementException nfe) {
throw new OptionsParsingException(
- "Expected exactly 2 comma-separated integer values", nfe);
+ "Expected even number of comma-separated integer values, could not parse integer in"
+ + " list",
+ nfe);
}
}
@Override
public String getTypeDescription() {
- return "two integers, separated by a comma";
+ return "integers, separated by a comma expected in pairs";
}
}
@Override
public String toString() {
- return MoreObjects.toStringHelper(this)
- .add("numTimesToDoGc", numTimesToDoGc)
- .add("timeToSleepBetweenGcs", timeToSleepBetweenGcs)
- .toString();
+ return MoreObjects.toStringHelper(this).add("gcSpecs", gcSpecs).toString();
}
}
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 356735c..bf962d3 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
@@ -337,9 +337,11 @@
effectTags = {OptionEffectTag.BAZEL_MONITORING},
converter = MemoryProfileStableHeapParameters.Converter.class,
help =
- "Tune memory profile's computation of stable heap at end of build. Should be two"
- + " integers separated by a comma. First parameter is the number of GCs to perform."
- + " Second parameter is the number of seconds to wait between GCs.")
+ "Tune memory profile's computation of stable heap at end of build. Should be and even"
+ + " number of integers separated by commas. In each pair the first integer is the"
+ + " number of GCs to perform. The second integer in each pair is the number of"
+ + " seconds to wait between GCs. Ex: 2,4,4,0 would 2 GCs with a 4sec pause, followed"
+ + " by 4 GCs with zero second pause")
public MemoryProfileStableHeapParameters memoryProfileStableHeapParameters;
@Option(
diff --git a/src/main/java/com/google/devtools/build/lib/util/BUILD b/src/main/java/com/google/devtools/build/lib/util/BUILD
index b86d8f3..e3cfc44 100644
--- a/src/main/java/com/google/devtools/build/lib/util/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/util/BUILD
@@ -143,6 +143,16 @@
)
java_library(
+ name = "pair",
+ srcs = [
+ "Pair.java",
+ ],
+ deps = [
+ "//third_party:jsr305",
+ ],
+)
+
+java_library(
name = "util",
srcs = [
"AbstractIndexer.java",
@@ -165,7 +175,6 @@
"OptionsUtils.java",
"OrderedSetMultimap.java",
"OsUtils.java",
- "Pair.java",
"PathFragmentFilter.java",
"PersistentMap.java",
"RegexFilter.java",
@@ -177,6 +186,10 @@
"TimeUtilities.java",
"UserUtils.java",
],
+ exports = [
+ # vfs depends on the profiler and creates a cycle since we use Pair in profiler
+ ":pair",
+ ],
deps = [
":os",
":shell_escaper",
