Automated rollback of commit 0bda661e589ded1caad9edd58c9bebc3f647e41d.
*** Reason for rollback ***
b/289354550
*** Original change description ***
Clean up Label Interner flag and relevant unused code
PiperOrigin-RevId: 544412858
Change-Id: Ibbdc6e0f0768702be236ab25c5622e226d524d25
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 0b8f66c..25e432e 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -19,6 +19,7 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Interner;
import com.google.common.util.concurrent.Striped;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.actions.CommandLineItem;
@@ -33,6 +34,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.UsePooledLabelInterningFlag;
import java.util.Arrays;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
@@ -84,10 +86,16 @@
public static final SkyFunctionName TRANSITIVE_TRAVERSAL =
SkyFunctionName.createHermetic("TRANSITIVE_TRAVERSAL");
- private static final LabelInterner interner = new LabelInterner();
+ @Nullable
+ private static final LabelInterner pooledInterner =
+ UsePooledLabelInterningFlag.usePooledLabelInterningFlag() ? new LabelInterner() : null;
+ private static final Interner<Label> interner =
+ pooledInterner != null ? pooledInterner : BlazeInterners.newWeakInterner();
+
+ @Nullable
public static LabelInterner getLabelInterner() {
- return interner;
+ return pooledInterner;
}
/** The context of a current repo, necessary to parse a repo-relative label ("//foo:bar"). */
diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD
index d9c920b..139223e 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/skyframe/BUILD
@@ -17,6 +17,7 @@
"SkyFunctionName.java",
"SkyKey.java",
"SkyValue.java",
+ "UsePooledLabelInterningFlag.java",
"Version.java",
]
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 ad90b06..2645ae1 100644
--- a/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
+++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java
@@ -80,7 +80,9 @@
this.usePooledInterning = usePooledInterning;
if (usePooledInterning) {
SkyKeyInterner.setGlobalPool(new SkyKeyPool());
- LabelInterner.setGlobalPool(new LabelPool());
+ if (UsePooledLabelInterningFlag.usePooledLabelInterningFlag()) {
+ LabelInterner.setGlobalPool(new LabelPool());
+ }
}
}
@@ -255,7 +257,9 @@
e -> {
weakInternSkyKey(e.getKey());
- if (!e.isDone() || !e.getKey().functionName().equals(SkyFunctions.PACKAGE)) {
+ if (!UsePooledLabelInterningFlag.usePooledLabelInterningFlag()
+ || !e.isDone()
+ || !e.getKey().functionName().equals(SkyFunctions.PACKAGE)) {
return;
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/UsePooledLabelInterningFlag.java b/src/main/java/com/google/devtools/build/skyframe/UsePooledLabelInterningFlag.java
new file mode 100644
index 0000000..124c66c
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/skyframe/UsePooledLabelInterningFlag.java
@@ -0,0 +1,44 @@
+// 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 com.google.devtools.build.lib.util.TestType;
+import java.util.Objects;
+
+/**
+ * A startup flag to decide whether some {@link com.google.devtools.build.lib.cmdline.Label}s will
+ * use {@link com.google.devtools.build.lib.cmdline.LabelInterner} backed by {@link
+ * com.google.devtools.build.lib.concurrent.PooledInterner} or just the regular bazel weak interner.
+ *
+ * <p>When this flag is true, {@code LabelInterner} will be applied for all {@code Label}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 LabelInterner} can reduce memory overhead of having duplicate {@code Label}
+ * instances in both weak interner and {@link InMemoryGraphImpl}.
+ */
+// TODO(b/250641010): This flag is temporary to facilitate a controlled rollout. So it should be
+// removed after the new pooled interning for `Label` instances is fully released and stable.
+public final class UsePooledLabelInterningFlag {
+
+ private static final boolean USE_POOLED_LABEL_INTERNER =
+ Objects.equals(System.getProperty("BAZEL_USE_POOLED_LABEL_INTERNER"), "1")
+ || TestType.isInTest();
+
+ public static boolean usePooledLabelInterningFlag() {
+ return USE_POOLED_LABEL_INTERNER;
+ }
+
+ private UsePooledLabelInterningFlag() {}
+}
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/BUILD b/src/test/java/com/google/devtools/build/lib/cmdline/BUILD
index 4c67386..94ea4e6 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/BUILD
@@ -50,6 +50,7 @@
java_test(
name = "LabelInternerIntegrationTest",
srcs = ["LabelInternerIntegrationTest.java"],
+ jvm_flags = ["-DBAZEL_USE_POOLED_LABEL_INTERNER=1"],
deps = [
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
diff --git a/src/test/java/com/google/devtools/build/skyframe/BUILD b/src/test/java/com/google/devtools/build/skyframe/BUILD
index 56a79a1..b508c52 100644
--- a/src/test/java/com/google/devtools/build/skyframe/BUILD
+++ b/src/test/java/com/google/devtools/build/skyframe/BUILD
@@ -87,6 +87,7 @@
java_test(
name = "SkyframeTests",
size = "medium",
+ jvm_flags = ["-DBAZEL_USE_POOLED_LABEL_INTERNER=1"],
shard_count = 2,
tags = ["not_run:arm"],
test_class = "com.google.devtools.build.skyframe.AllTests",
