Remove blaze start-up flag `UsePooledSkyKeyInterningFlag`.
This commit also fixes some trivial typos in shell integration tests.
PiperOrigin-RevId: 518379794
Change-Id: Iee6d06afb9468cc9d614610cd1719864dfe3fdd1
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 1549b80..8bc5e72 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -226,7 +226,8 @@
inconsistencyReceiver,
trackIncrementalState ? DEFAULT_EVENT_FILTER_WITH_ACTIONS : EventFilter.NO_STORAGE,
emittedEventState,
- trackIncrementalState);
+ trackIncrementalState,
+ /* usePooledSkyKeyInterning= */ true);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index 49d56a3..63c06e3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -141,6 +141,7 @@
private final int nonSkyframeGlobbingThreads;
@VisibleForTesting final ForkJoinPool forkJoinPoolForNonSkyframeGlobbing;
private final int skyframeThreads;
+ private final boolean usePooledSkyKeyInterning;
/** Abstract base class of a builder for {@link PackageLoader} instances. */
public abstract static class Builder {
@@ -157,6 +158,7 @@
List<PrecomputedValue.Injected> extraPrecomputedValues = new ArrayList<>();
int nonSkyframeGlobbingThreads = 1;
int skyframeThreads = 1;
+ boolean usePooledSkyKeyInterning = true;
protected Builder(
Root workspaceDir,
@@ -242,6 +244,12 @@
return this;
}
+ @CanIgnoreReturnValue
+ public Builder disablePooledSkyKeyInterning() {
+ this.usePooledSkyKeyInterning = false;
+ return this;
+ }
+
/** Throws {@link IllegalArgumentException} if builder args are incomplete/inconsistent. */
protected void validate() {
if (starlarkSemantics == null) {
@@ -273,6 +281,7 @@
NamedForkJoinPool.newNamedPool(
"package-loader-globbing-pool", builder.nonSkyframeGlobbingThreads);
this.skyframeThreads = builder.skyframeThreads;
+ this.usePooledSkyKeyInterning = builder.usePooledSkyKeyInterning;
this.directories = builder.directories;
this.hashFunction = builder.workspaceDir.getFileSystem().getDigestFunction().getHashFunction();
@@ -408,7 +417,8 @@
GraphInconsistencyReceiver.THROWING,
EventFilter.FULL_STORAGE,
new NestedSetVisitor.VisitedState(),
- /*keepEdges=*/ false);
+ /* keepEdges= */ false,
+ usePooledSkyKeyInterning);
}
protected abstract ImmutableList<EnvironmentExtension> getEnvironmentExtensions();
diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD
index 130c650..eb0b4ce 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/skyframe/BUILD
@@ -16,7 +16,6 @@
"SkyFunctionName.java",
"SkyKey.java",
"SkyValue.java",
- "UsePooledSkyKeyInterningFlag.java",
"Version.java",
]
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
index 3fe469a..bb67bd2 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraph.java
@@ -26,15 +26,19 @@
/** Creates a new in-memory graph suitable for incremental builds. */
static InMemoryGraph create() {
- return new InMemoryGraphImpl();
+ return new InMemoryGraphImpl(/* usePooledSkyKeyInterning= */ true);
+ }
+
+ static InMemoryGraph create(boolean usePooledSkyKeyInterning) {
+ return new InMemoryGraphImpl(usePooledSkyKeyInterning);
}
/**
* Creates a new in-memory graph that discards graph edges to save memory and cannot be used for
* incremental builds.
*/
- static InMemoryGraph createEdgeless() {
- return new EdgelessInMemoryGraphImpl();
+ static InMemoryGraph createEdgeless(boolean usePooledSkyKeyInterning) {
+ return new EdgelessInMemoryGraphImpl(usePooledSkyKeyInterning);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
index 0c44441..4965a39 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
@@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
@@ -44,21 +43,22 @@
protected final ConcurrentHashMap<SkyKey, InMemoryNodeEntry> nodeMap;
private final NodeBatch getBatch;
private final NodeBatch createIfAbsentBatch;
-
- // TODO(b/250641010): Remove this class member along with the startup flag.
private final boolean usePooledSkyKeyInterning;
InMemoryGraphImpl() {
this(/* initialCapacity= */ 1 << 10);
}
- @VisibleForTesting
- public InMemoryGraphImpl(boolean usePooledSkyKeyInterning) {
+ /**
+ * For some shell integration tests, we don't want to apply {@link SkyKeyInterner} created and
+ * bind {@code SkyKeyInterner#globalPool} to the second {@link InMemoryGraph}.
+ */
+ InMemoryGraphImpl(boolean usePooledSkyKeyInterning) {
this(/* initialCapacity= */ 1 << 10, usePooledSkyKeyInterning);
}
protected InMemoryGraphImpl(int initialCapacity) {
- this(initialCapacity, UsePooledSkyKeyInterningFlag.usePooledSkyKeyInterningFlag());
+ this(initialCapacity, /* usePooledSkyKeyInterning= */ true);
}
private InMemoryGraphImpl(int initialCapacity, boolean usePooledSkyKeyInterning) {
@@ -217,7 +217,7 @@
@Override
public void cleanupPool() {
if (!usePooledSkyKeyInterning) {
- // No clean up is needed when UseSkyKeyInternerFlag startup flag is off.
+ // No clean up is needed when `usePooledSkyKeyInterning` is false for shell integration tests.
return;
}
try (AutoProfiler ignored =
@@ -231,11 +231,6 @@
static final class EdgelessInMemoryGraphImpl extends InMemoryGraphImpl {
- public EdgelessInMemoryGraphImpl() {
- super();
- }
-
- @VisibleForTesting
public EdgelessInMemoryGraphImpl(boolean usePooledSkyKeyInterning) {
super(usePooledSkyKeyInterning);
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
index f0ebf4d..1081fac 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java
@@ -92,7 +92,8 @@
GraphInconsistencyReceiver.THROWING,
EventFilter.FULL_STORAGE,
new NestedSetVisitor.VisitedState(),
- /*keepEdges=*/ true);
+ /* keepEdges= */ true,
+ /* usePooledSkyKeyInterning= */ true);
}
public InMemoryMemoizingEvaluator(
@@ -102,13 +103,17 @@
GraphInconsistencyReceiver graphInconsistencyReceiver,
EventFilter eventFilter,
NestedSetVisitor.VisitedState emittedEventState,
- boolean keepEdges) {
+ boolean keepEdges,
+ boolean usePooledSkyKeyInterning) {
this.skyFunctions = ImmutableMap.copyOf(skyFunctions);
this.differencer = Preconditions.checkNotNull(differencer);
this.progressReceiver = new DirtyTrackingProgressReceiver(progressReceiver);
this.graphInconsistencyReceiver = Preconditions.checkNotNull(graphInconsistencyReceiver);
this.eventFilter = eventFilter;
- this.graph = keepEdges ? InMemoryGraph.create() : InMemoryGraph.createEdgeless();
+ this.graph =
+ keepEdges
+ ? InMemoryGraph.create(usePooledSkyKeyInterning)
+ : InMemoryGraph.createEdgeless(usePooledSkyKeyInterning);
this.emittedEventState = emittedEventState;
this.keepEdges = keepEdges;
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/UsePooledSkyKeyInterningFlag.java b/src/main/java/com/google/devtools/build/skyframe/UsePooledSkyKeyInterningFlag.java
deleted file mode 100644
index 3178703..0000000
--- a/src/main/java/com/google/devtools/build/skyframe/UsePooledSkyKeyInterningFlag.java
+++ /dev/null
@@ -1,41 +0,0 @@
-// Copyright 2023 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.skyframe;
-
-import java.util.Objects;
-
-/**
- * A startup flag to decide whether some {@link SkyKey}s will use {@code SkyKeyInterner} or just the
- * regular bazel weak interner.
- *
- * <p>When this flag is true, {@code SkyKeyInterner} will be applied for some {@link SkyKey}s so
- * that they are able to switch between interning the instances between the regular bazel weak
- * interner and the static global pool.
- *
- * <p>Applying {@code SkyKeyInterner} can reduce memory overhead of having duplicate {@link SkyKey}
- * instances in both weak interner and some other storage.
- */
-// TODO(b/250641010): This flag is temporary to facilitate a controlled rollout. So it should be
-// removed after the new pooled interning is fully released and stable.
-public final class UsePooledSkyKeyInterningFlag {
-
- private static final boolean USE_POOLED_SKY_KEY_INTERNER =
- Objects.equals(System.getProperty("BAZEL_USE_POOLED_SKY_KEY_INTERNER"), "1");
-
- public static boolean usePooledSkyKeyInterningFlag() {
- return USE_POOLED_SKY_KEY_INTERNER;
- }
-
- private UsePooledSkyKeyInterningFlag() {}
-}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index b4d8cd5..546a225 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -275,7 +275,7 @@
null,
null,
null,
- /*packageProgress=*/ null,
+ /* packageProgress= */ null,
PackageFunction.ActionOnIOExceptionReadingBuildFile.UseOriginalIOException
.INSTANCE,
GlobbingStrategy.SKYFRAME_HYBRID,
@@ -295,7 +295,7 @@
.builder(directories)
.build(TestRuleClassProvider.getRuleClassProvider(), fileSystem),
directories,
- /*bzlLoadFunctionForInlining=*/ null))
+ /* bzlLoadFunctionForInlining= */ null))
.put(
SkyFunctions.EXTERNAL_PACKAGE,
new ExternalPackageFunction(
@@ -306,14 +306,15 @@
.put(
SkyFunctions.ARTIFACT_NESTED_SET,
ArtifactNestedSetFunction.createInstance(
- /*valueBasedChangePruningEnabled=*/ true))
+ /* valueBasedChangePruningEnabled= */ true))
.buildOrThrow(),
differencer,
evaluationProgressReceiver,
graphInconsistencyReceiver,
EventFilter.FULL_STORAGE,
new NestedSetVisitor.VisitedState(),
- /*keepEdges=*/ true);
+ /* keepEdges= */ true,
+ /* usePooledSkyKeyInterning= */ true);
PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
PrecomputedValue.ACTION_ENV.set(differencer, ImmutableMap.of());
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java
index 4540a98..77b0d66 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryGraphTest.java
@@ -38,7 +38,7 @@
@Override
protected void makeGraph() {
- graph = new InMemoryGraphImpl(/* usePooledSkyKeyInterning= */ true);
+ graph = new InMemoryGraphImpl();
}
@Override
diff --git a/src/test/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluatorTest.java
index 685049c..17975b4 100644
--- a/src/test/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluatorTest.java
@@ -35,6 +35,7 @@
graphInconsistencyReceiver,
eventFilter,
emittedEventState,
- /*keepEdges=*/ true);
+ /* keepEdges= */ true,
+ /* usePooledSkyKeyInterning= */ true);
}
}
diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
index 9e6bc63..f3b6572 100644
--- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java
@@ -1004,7 +1004,10 @@
throws Exception {
Assume.assumeTrue(keepGoing || keepEdges);
- graph = keepEdges ? InMemoryGraph.create() : InMemoryGraph.createEdgeless();
+ graph =
+ keepEdges
+ ? InMemoryGraph.create(/* usePooledSkyKeyInterning= */ true)
+ : InMemoryGraph.createEdgeless(/* usePooledSkyKeyInterning= */ true);
SkyKey catastropheKey = GraphTester.toSkyKey("catastrophe");
SkyKey otherKey = GraphTester.toSkyKey("someKey");
diff --git a/src/test/shell/integration/discard_graph_edges_test.sh b/src/test/shell/integration/discard_graph_edges_test.sh
index d9d1019..07781db 100755
--- a/src/test/shell/integration/discard_graph_edges_test.sh
+++ b/src/test/shell/integration/discard_graph_edges_test.sh
@@ -260,7 +260,7 @@
local edgeless_entry_count="$(extract_histogram_count "$histo_file" \
'EdgelessInMemoryNodeEntry')"
[[ "$edgeless_entry_count" -eq 0 ]] \
- || fail "$edgless_entry_count EdgelessInMemoryNodeEntry instances found in build keeping edges"
+ || fail "$edgeless_entry_count EdgelessInMemoryNodeEntry instances found in build keeping edges"
local node_entry_count="$(extract_histogram_count "$histo_file" \
'\.InMemoryNodeEntry')"
[[ "$node_entry_count" -ge 100 ]] \
diff --git a/src/test/shell/integration/starlark_configurations_test.sh b/src/test/shell/integration/starlark_configurations_test.sh
index fb83279..37a8add 100755
--- a/src/test/shell/integration/starlark_configurations_test.sh
+++ b/src/test/shell/integration/starlark_configurations_test.sh
@@ -112,8 +112,8 @@
#### TESTS #############################################################
function test_default_flag() {
- local -r pkg=$FUNCNAME
- mkdir -p $pkg
+ local -r pkg=$FUNCNAME
+ mkdir -p $pkg
write_build_setting_bzl
