Skip to content

Commit ea75928

Browse files
AlessandroPatticopybara-github
AlessandroPatti
authored and
copybara-github
committed
Introduce --local_resources flag
Follow up for #19839 (comment), introduce new flag that will replace all the others. Closes #20398. PiperOrigin-RevId: 596905096 Change-Id: Ic2c20dae40efa797953cde6934085f899acb9b52
1 parent 0ad0506 commit ea75928

File tree

17 files changed

+262
-173
lines changed

17 files changed

+262
-173
lines changed

‎src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.devtools.build.lib.util.CpuResourceConverter;
1818
import com.google.devtools.build.lib.util.RegexFilter;
19+
import com.google.devtools.build.lib.util.ResourceConverter;
1920
import com.google.devtools.common.options.Option;
2021
import com.google.devtools.common.options.OptionDocumentationCategory;
2122
import com.google.devtools.common.options.OptionEffectTag;
@@ -106,24 +107,29 @@ public class AnalysisOptions extends OptionsBase {
106107

107108
@Option(
108109
name = "experimental_skyframe_cpu_heavy_skykeys_thread_pool_size",
109-
defaultValue = "HOST_CPUS",
110+
defaultValue = ResourceConverter.HOST_CPUS_KEYWORD,
110111
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
111112
metadataTags = OptionMetadataTag.EXPERIMENTAL,
112113
effectTags = {
113114
OptionEffectTag.LOADING_AND_ANALYSIS,
114115
OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION
115116
},
116117
help =
117-
"If set to a positive value (e.g. \"HOST_CPUS*1.5\"), Skyframe will run the"
118-
+ " loading/analysis phase with 2 separate thread pools: 1 with <value> threads"
119-
+ " (ideally close to HOST_CPUS) reserved for CPU-heavy SkyKeys, and 1 \"standard\""
118+
"If set to a positive value (e.g. \""
119+
+ ResourceConverter.HOST_CPUS_KEYWORD
120+
+ "*1.5\"),"
121+
+ " Skyframe will run the loading/analysis phase with 2 separate thread pools:"
122+
+ " 1 with <value> threads (ideally close to "
123+
+ ResourceConverter.HOST_CPUS_KEYWORD
124+
+ ")"
125+
+ " reserved for CPU-heavy SkyKeys, and 1 \"standard\""
120126
+ " thread pool (whose size is controlled by --loading_phase_threads) for the rest.",
121127
converter = CpuResourceConverter.class)
122128
public int cpuHeavySkyKeysThreadPoolSize;
123129

124130
@Option(
125131
name = "experimental_oom_sensitive_skyfunctions_semaphore_size",
126-
defaultValue = "HOST_CPUS",
132+
defaultValue = ResourceConverter.HOST_CPUS_KEYWORD,
127133
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
128134
metadataTags = OptionMetadataTag.EXPERIMENTAL,
129135
effectTags = {
@@ -133,7 +139,9 @@ public class AnalysisOptions extends OptionsBase {
133139
help =
134140
"Sets the size of the semaphore used to prevent SkyFunctions with large peak memory"
135141
+ " requirement from OOM-ing blaze. A value of 0 indicates that no semaphore should"
136-
+ " be used. Example value: \"HOST_CPUS*0.5\".",
142+
+ " be used. Example value: \""
143+
+ ResourceConverter.HOST_CPUS_KEYWORD
144+
+ "*0.5\".",
137145
converter = CpuResourceConverter.class)
138146
public int oomSensitiveSkyFunctionsSemaphoreSize;
139147
}

‎src/main/java/com/google/devtools/build/lib/analysis/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ java_library(
472472
deps = [
473473
"//src/main/java/com/google/devtools/build/lib/util",
474474
"//src/main/java/com/google/devtools/build/lib/util:cpu_resource_converter",
475+
"//src/main/java/com/google/devtools/build/lib/util:resource_converter",
475476
"//src/main/java/com/google/devtools/common/options",
476477
],
477478
)

‎src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import com.github.benmanes.caffeine.cache.CaffeineSpec;
1717
import com.google.common.annotations.VisibleForTesting;
1818
import com.google.common.flogger.GoogleLogger;
19-
import com.google.devtools.build.lib.actions.LocalHostCapacity;
2019
import com.google.devtools.build.lib.util.OptionsUtils;
2120
import com.google.devtools.build.lib.util.ResourceConverter;
2221
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -397,13 +396,9 @@ public String getSymlinkPrefix(String productName) {
397396
public int skymeldAnalysisOverlapPercentage;
398397

399398
/** Converter for filesystem value checker threads. */
400-
public static class ThreadConverter extends ResourceConverter {
399+
public static class ThreadConverter extends ResourceConverter.IntegerConverter {
401400
public ThreadConverter() {
402-
super(
403-
/* autoSupplier= */ () ->
404-
(int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()),
405-
/* minValue= */ 1,
406-
/* maxValue= */ Integer.MAX_VALUE);
401+
super(/* auto= */ HOST_CPUS_SUPPLIER, /* minValue= */ 1, /* maxValue= */ Integer.MAX_VALUE);
407402
}
408403
}
409404

@@ -446,27 +441,24 @@ public ThreadConverter() {
446441
* Converter for jobs: Takes keyword ({@value #FLAG_SYNTAX}). Values must be between 1 and
447442
* MAX_JOBS.
448443
*/
449-
public static class JobsConverter extends ResourceConverter {
444+
public static class JobsConverter extends ResourceConverter.IntegerConverter {
450445
public JobsConverter() {
451-
super(
452-
() -> (int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()),
453-
1,
454-
MAX_JOBS);
446+
super(/* auto= */ HOST_CPUS_SUPPLIER, /* minValue= */ 1, /* maxValue= */ MAX_JOBS);
455447
}
456448

457449
@Override
458-
public int checkAndLimit(int value) throws OptionsParsingException {
459-
if (value < minValue) {
450+
public Integer checkAndLimit(Integer value) throws OptionsParsingException {
451+
if (value.doubleValue() < minValue) {
460452
throw new OptionsParsingException(
461453
String.format("Value '(%d)' must be at least %d.", value, minValue));
462454
}
463-
if (value > maxValue) {
455+
if (value.doubleValue() > maxValue) {
464456
logger.atWarning().log(
465457
"Flag remoteWorker \"jobs\" ('%d') was set too high. "
466458
+ "This is a result of passing large values to --local_resources or --jobs. "
467459
+ "Using '%d' jobs",
468460
value, maxValue);
469-
value = maxValue;
461+
return maxValue;
470462
}
471463
return value;
472464
}

‎src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.google.common.collect.ImmutableSet;
2525
import com.google.common.collect.Iterables;
2626
import com.google.common.collect.Sets;
27+
import com.google.common.collect.Streams;
2728
import com.google.common.eventbus.Subscribe;
2829
import com.google.common.flogger.GoogleLogger;
2930
import com.google.devtools.build.lib.actions.Action;
@@ -115,7 +116,6 @@
115116
import java.util.Set;
116117
import java.util.UUID;
117118
import java.util.concurrent.atomic.AtomicBoolean;
118-
import java.util.stream.Stream;
119119
import javax.annotation.Nullable;
120120

121121
/**
@@ -996,11 +996,16 @@ public static void configureResourceManager(ResourceManager resourceMgr, BuildRe
996996
ImmutableMap<String, Double> cpuRam =
997997
ImmutableMap.of(
998998
ResourceSet.CPU,
999+
// Replace with 1.0 * ResourceConverter.HOST_CPUS.get() after flag deprecation
9991000
options.localCpuResources,
10001001
ResourceSet.MEMORY,
1002+
// Replace with 0.67 * ResourceConverter.HOST_RAM.get() after flag deprecation
10011003
options.localRamResources);
10021004
ImmutableMap<String, Double> resources =
1003-
Stream.concat(options.localExtraResources.stream(), cpuRam.entrySet().stream())
1005+
Streams.concat(
1006+
options.localExtraResources.stream(),
1007+
cpuRam.entrySet().stream(),
1008+
options.localResources.stream())
10041009
.collect(
10051010
ImmutableMap.toImmutableMap(
10061011
Map.Entry::getKey, Map.Entry::getValue, (v1, v2) -> v2));

‎src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java‎

Lines changed: 56 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -293,35 +293,54 @@ public boolean shouldMaterializeParamFiles() {
293293

294294
@Option(
295295
name = "local_cpu_resources",
296-
defaultValue = "HOST_CPUS",
296+
defaultValue = ResourceConverter.HOST_CPUS_KEYWORD,
297+
deprecationWarning =
298+
"--local_cpu_resources is deprecated, please use --local_resources=cpu= instead.",
297299
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
298300
effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
299301
help =
300302
"Explicitly set the total number of local CPU cores available to Bazel to spend on build"
301-
+ " actions executed locally. Takes an integer, or \"HOST_CPUS\", optionally followed"
302-
+ " by [-|*]<float> (eg. HOST_CPUS*.5 to use half the available CPU cores).By"
303-
+ " default, (\"HOST_CPUS\"), Bazel will query system configuration to estimate"
304-
+ " the number of CPU cores available.",
303+
+ " actions executed locally. Takes an integer, or \""
304+
+ ResourceConverter.HOST_CPUS_KEYWORD
305+
+ "\", optionally followed"
306+
+ " by [-|*]<float> (eg. "
307+
+ ResourceConverter.HOST_CPUS_KEYWORD
308+
+ "*.5"
309+
+ " to use half the available CPU cores). By default, (\""
310+
+ ResourceConverter.HOST_CPUS_KEYWORD
311+
+ "\"), Bazel will query system"
312+
+ " configuration to estimate the number of CPU cores available.",
305313
converter = CpuResourceConverter.class)
306314
public double localCpuResources;
307315

308316
@Option(
309317
name = "local_ram_resources",
310-
defaultValue = "HOST_RAM*.67",
318+
defaultValue = ResourceConverter.HOST_RAM_KEYWORD + "*.67",
319+
deprecationWarning =
320+
"--local_ram_resources is deprecated, please use --local_resources=memory= instead.",
311321
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
312322
effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
313323
help =
314324
"Explicitly set the total amount of local host RAM (in MB) available to Bazel to spend on"
315-
+ " build actions executed locally. Takes an integer, or \"HOST_RAM\", optionally"
316-
+ " followed by [-|*]<float> (eg. HOST_RAM*.5 to use half the available RAM). By"
317-
+ " default, (\"HOST_RAM*.67\"), Bazel will query system configuration to estimate"
318-
+ " the amount of RAM available and will use 67% of it.",
325+
+ " build actions executed locally. Takes an integer, or \""
326+
+ ResourceConverter.HOST_RAM_KEYWORD
327+
+ "\", optionally followed by [-|*]<float>"
328+
+ " (eg. "
329+
+ ResourceConverter.HOST_RAM_KEYWORD
330+
+ "*.5 to use half the available"
331+
+ " RAM). By default, (\""
332+
+ ResourceConverter.HOST_RAM_KEYWORD
333+
+ "*.67\"),"
334+
+ " Bazel will query system configuration to estimate the amount of RAM available"
335+
+ " and will use 67% of it.",
319336
converter = RamResourceConverter.class)
320337
public double localRamResources;
321338

322339
@Option(
323340
name = "local_extra_resources",
324341
defaultValue = "null",
342+
deprecationWarning =
343+
"--local_extra_resources is deprecated, please use --local_resources instead.",
325344
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
326345
effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
327346
allowMultiple = true,
@@ -336,6 +355,31 @@ public boolean shouldMaterializeParamFiles() {
336355
converter = Converters.StringToDoubleAssignmentConverter.class)
337356
public List<Map.Entry<String, Double>> localExtraResources;
338357

358+
@Option(
359+
name = "local_resources",
360+
defaultValue = "null",
361+
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
362+
effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
363+
allowMultiple = true,
364+
help =
365+
"Set the number of resources available to Bazel. "
366+
+ "Takes in an assignment to a float or "
367+
+ ResourceConverter.HOST_RAM_KEYWORD
368+
+ "/"
369+
+ ResourceConverter.HOST_CPUS_KEYWORD
370+
+ ", optionally "
371+
+ "followed by [-|*]<float> (eg. memory="
372+
+ ResourceConverter.HOST_RAM_KEYWORD
373+
+ "*.5 to use half the available RAM). "
374+
+ "Can be used multiple times to specify multiple "
375+
+ "types of resources. Bazel will limit concurrently running actions "
376+
+ "based on the available resources and the resources required. "
377+
+ "Tests can declare the amount of resources they need "
378+
+ "by using a tag of the \"resources:<resource name>:<amount>\" format. "
379+
+ "Overrides resources specified by --local_{cpu|ram|extra}_resources.",
380+
converter = ResourceConverter.AssignmentConverter.class)
381+
public List<Map.Entry<String, Double>> localResources;
382+
339383
@Option(
340384
name = "local_test_jobs",
341385
defaultValue = "auto",
@@ -585,9 +629,9 @@ public String getTypeDescription() {
585629
}
586630

587631
/** Converter for --local_test_jobs, which takes {@value FLAG_SYNTAX} */
588-
public static class LocalTestJobsConverter extends ResourceConverter {
632+
public static class LocalTestJobsConverter extends ResourceConverter.IntegerConverter {
589633
public LocalTestJobsConverter() throws OptionsParsingException {
590-
super(/* autoSupplier= */ () -> 0, /* minValue= */ 0, /* maxValue= */ Integer.MAX_VALUE);
634+
super(/* auto= */ () -> 0, /* minValue= */ 0, /* maxValue= */ Integer.MAX_VALUE);
591635
}
592636
}
593637

‎src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningOptions.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.includescanning;
1515

16-
import com.google.devtools.build.lib.actions.LocalHostCapacity;
1716
import com.google.devtools.build.lib.util.ResourceConverter;
1817
import com.google.devtools.common.options.Option;
1918
import com.google.devtools.common.options.OptionDocumentationCategory;
@@ -30,13 +29,9 @@ public class IncludeScanningOptions extends OptionsBase {
3029
* Converter for scanning parallelism threads: Takes {@value #FLAG_SYNTAX} 0 disables scanning
3130
* parallelism.
3231
*/
33-
public static class ParallelismConverter extends ResourceConverter {
32+
public static class ParallelismConverter extends ResourceConverter.IntegerConverter {
3433
public ParallelismConverter() throws OptionsParsingException {
35-
super(
36-
/* autoSupplier= */ () ->
37-
(int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()),
38-
/* minValue= */ 0,
39-
/* maxValue= */ Integer.MAX_VALUE);
34+
super(/* auto= */ HOST_CPUS_SUPPLIER, /* minValue= */ 0, /* maxValue= */ Integer.MAX_VALUE);
4035
}
4136
}
4237

‎src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.google.common.base.Strings;
2020
import com.google.common.collect.ImmutableList;
2121
import com.google.common.collect.ImmutableSet;
22-
import com.google.devtools.build.lib.actions.LocalHostCapacity;
2322
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
2423
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
2524
import com.google.devtools.build.lib.packages.RuleVisibility;
@@ -59,13 +58,9 @@ public String getTypeDescription() {
5958
}
6059

6160
/** Converter for globbing threads. */
62-
public static class ParallelismConverter extends ResourceConverter {
61+
public static class ParallelismConverter extends ResourceConverter.IntegerConverter {
6362
public ParallelismConverter() throws OptionsParsingException {
64-
super(
65-
/* autoSupplier= */ () ->
66-
(int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()),
67-
/* minValue= */ 1,
68-
/* maxValue= */ Integer.MAX_VALUE);
63+
super(/* auto= */ HOST_CPUS_SUPPLIER, /* minValue= */ 1, /* maxValue= */ Integer.MAX_VALUE);
6964
}
7065
}
7166

‎src/main/java/com/google/devtools/build/lib/runtime/LoadingPhaseThreadsOption.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package com.google.devtools.build.lib.runtime;
1515

1616
import com.google.common.flogger.GoogleLogger;
17-
import com.google.devtools.build.lib.actions.LocalHostCapacity;
1817
import com.google.devtools.build.lib.util.ResourceConverter;
1918
import com.google.devtools.build.lib.util.TestType;
2019
import com.google.devtools.common.options.Option;
@@ -45,18 +44,19 @@ public class LoadingPhaseThreadsOption extends OptionsBase {
4544
/**
4645
* A converter for loading phase thread count. Takes {@value FLAG_SYNTAX}. Caps at 20 for tests.
4746
*/
48-
public static final class LoadingPhaseThreadCountConverter extends ResourceConverter {
47+
public static final class LoadingPhaseThreadCountConverter
48+
extends ResourceConverter.IntegerConverter {
4949

5050
public LoadingPhaseThreadCountConverter() {
5151
// TODO(jmmv): Using the number of cores has proven to yield reasonable analysis times on
5252
// Mac Pros and MacBook Pros but we should probably do better than this. (We haven't made
5353
// any guarantees that "auto" means number of cores precisely to leave us room to tune this
5454
// further in the future.)
55-
super(() -> (int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()));
55+
super(/* auto= */ HOST_CPUS_SUPPLIER, /* minValue= */ 1, /* maxValue= */ Integer.MAX_VALUE);
5656
}
5757

5858
@Override
59-
public int checkAndLimit(int value) throws OptionsParsingException {
59+
public Integer checkAndLimit(Integer value) throws OptionsParsingException {
6060
// Cap thread count while running tests. Test cases are typically small and large thread
6161
// pools vying for a relatively small number of CPU cores may induce non-optimal
6262
// performance.

‎src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import com.google.common.collect.ImmutableSet;
1919
import com.google.common.collect.Lists;
2020
import com.google.common.collect.Maps;
21-
import com.google.devtools.build.lib.actions.LocalHostCapacity;
2221
import com.google.devtools.build.lib.util.OptionsUtils;
2322
import com.google.devtools.build.lib.util.RamResourceConverter;
2423
import com.google.devtools.build.lib.util.ResourceConverter;
@@ -371,12 +370,9 @@ public ImmutableSet<Path> getInaccessiblePaths(FileSystem fs) {
371370
public int memoryLimitMb;
372371

373372
/** Converter for the number of threads used for asynchronous tree deletion. */
374-
public static final class AsyncTreeDeletesConverter extends ResourceConverter {
373+
public static final class AsyncTreeDeletesConverter extends ResourceConverter.IntegerConverter {
375374
public AsyncTreeDeletesConverter() {
376-
super(
377-
() -> (int) Math.ceil(LocalHostCapacity.getLocalHostCapacity().getCpuUsage()),
378-
0,
379-
Integer.MAX_VALUE);
375+
super(/* auto= */ HOST_CPUS_SUPPLIER, /* minValue= */ 0, /* maxValue= */ Integer.MAX_VALUE);
380376
}
381377
}
382378
}

‎src/main/java/com/google/devtools/build/lib/util/BUILD‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ java_library(
128128
deps = [
129129
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
130130
"//src/main/java/com/google/devtools/common/options",
131+
"//third_party:error_prone_annotations",
131132
"//third_party:guava",
132133
"//third_party:jsr305",
133134
],
@@ -140,7 +141,6 @@ java_library(
140141
],
141142
deps = [
142143
":resource_converter",
143-
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
144144
"//third_party:guava",
145145
],
146146
)
@@ -152,7 +152,6 @@ java_library(
152152
],
153153
deps = [
154154
":resource_converter",
155-
"//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity",
156155
"//third_party:guava",
157156
],
158157
)

0 commit comments

Comments
 (0)