Dedupe ImmutableLists created for attribute values at the Starlark -> Blaze barrier during BUILD file evaluation.
We dedupe across all lists across all attributes of all rule instances in a single package. It's very common for multiple rules to have attributes that happen to have the same value. Blaze currently unconditionally creates a shallow copy of each list-typed attribute value, which means there's not even a way for users to write optimized BUILD/bzl code. Assume we have
def f(x):
...
# assume 'f' has no side effects
and then consider the following BUILD/bzl code
sh_library(name = 't1', srcs = ['f1.sh'], deps = f(42))
sh_library(name = 't2', srcs = ['f2.sh'], deps = f(42))
A thoughtful user might hope/assume Blaze optimizes this and only evaluates 'f(42)' once and only has one copy of the result of 'f(42)' in memory. Both of these thoughts are false.
So the thoughtful user might then write this "optimized" code instead
L = f(42)
sh_library(name = 't1', srcs = ['f1.sh'], deps = L)
sh_library(name = 't2', srcs = ['f2.sh'], deps = L)
here, 'f(42)' will be evaluated once. but Blaze internally still has two copies of L!
There are some good reasons that Blaze unconditionally creates copies of lists. One of them is that Starlark lists are mutable. Consider
L = f(42)
g(L) # assume 'g' has side-effects, e.g. rule instantiation
L.append('blah')
g(L)
Here, the first call to 'g' definitely needs to use the initial value of L.
But, not all BUILD/bzl code is like this and it's possibly to statically determine we're in the situation where L is *not* mutated [1], and so therefore it's possible to automatically share L in an optimizing manner.
[1] To be clear, I mean "it's possible to statically determine which of the 2 following situations we're in: (i) L is *definitely not* mutated or (ii) L *might be* mutated". This entails things like alias analysis too.
RELNOTES: None
PiperOrigin-RevId: 231239991
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 739360a..1af192c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -28,6 +28,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Interner;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
@@ -1846,7 +1847,11 @@
throws InterruptedException, CannotPrecomputeDefaultsException {
BitSet definedAttrIndices =
populateDefinedRuleAttributeValues(
- rule, pkgBuilder.getRepositoryMapping(), attributeValues, eventHandler);
+ rule,
+ pkgBuilder.getRepositoryMapping(),
+ attributeValues,
+ pkgBuilder.getListInterner(),
+ eventHandler);
populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler);
// Now that all attributes are bound to values, collect and store configurable attribute keys.
populateConfigDependenciesAttribute(rule);
@@ -1867,6 +1872,7 @@
Rule rule,
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
AttributeValues<T> attributeValues,
+ Interner<ImmutableList<?>> listInterner,
EventHandler eventHandler) {
BitSet definedAttrIndices = new BitSet();
for (T attributeAccessor : attributeValues.getAttributeAccessors()) {
@@ -1893,7 +1899,7 @@
if (attributeValues.valuesAreBuildLanguageTyped()) {
try {
nativeAttributeValue =
- convertFromBuildLangType(rule, attr, attributeValue, repositoryMapping);
+ convertFromBuildLangType(rule, attr, attributeValue, repositoryMapping, listInterner);
} catch (ConversionException e) {
rule.reportError(String.format("%s: %s", rule.getLabel(), e.getMessage()), eventHandler);
continue;
@@ -2193,7 +2199,8 @@
Rule rule,
Attribute attr,
Object buildLangValue,
- ImmutableMap<RepositoryName, RepositoryName> repositoryMapping)
+ ImmutableMap<RepositoryName, RepositoryName> repositoryMapping,
+ Interner<ImmutableList<?>> listInterner)
throws ConversionException {
LabelConversionContext context = new LabelConversionContext(rule.getLabel(), repositoryMapping);
Object converted =
@@ -2214,7 +2221,10 @@
List<? extends Comparable<?>> list = (List<? extends Comparable<?>>) converted;
converted = Ordering.natural().sortedCopy(list);
}
- converted = ImmutableList.copyOf((List<?>) converted);
+ // It's common for multiple rule instances in the same package to have the same value for some
+ // attributes. As a concrete example, consider a package having several 'java_test' instances,
+ // each with the same exact 'tags' attribute value.
+ converted = listInterner.intern(ImmutableList.copyOf((List<?>) converted));
}
return converted;
