Improve memory footprint of CcToolchainFeatures.Variables.
--
MOS_MIGRATED_REVID=140131142
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
index 694f4e6..83bcf05 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java
@@ -29,7 +29,6 @@
import com.google.common.collect.Sets;
import com.google.common.collect.Sets.SetView;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariableValue;
import com.google.devtools.build.lib.util.Pair;
@@ -972,17 +971,13 @@
@Immutable
private static final class StringSequence implements VariableValue {
- private final Iterable<String> values;
+ private final ImmutableList<String> values;
public StringSequence(ImmutableList<String> values) {
Preconditions.checkNotNull(values, "Cannot create StringSequence from null");
this.values = values;
}
- StringSequence(NestedSet<String> values) {
- this.values = values;
- }
-
@Override
public Iterable<? extends VariableValue> getSequenceValue(String variableName) {
final ImmutableList.Builder<VariableValue> sequences = ImmutableList.builder();
@@ -1149,30 +1144,27 @@
*/
public static class Builder {
private final Map<String, VariableValue> variablesMap = new LinkedHashMap<>();
+ private final Map<String, String> stringVariablesMap = new LinkedHashMap<>();
/** Add a variable that expands {@code name} to {@code value}. */
public Builder addStringVariable(String name, String value) {
Preconditions.checkArgument(
!variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
+ Preconditions.checkArgument(
+ !stringVariablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
Preconditions.checkNotNull(
value, "Cannot set null as a value for variable '%s'", name);
- variablesMap.put(name, new StringValue(value));
+ stringVariablesMap.put(name, value);
return this;
}
/** Add a sequence variable that expands {@code name} to {@code values}. */
- public Builder addStringSequenceVariable(String name, ImmutableList<String> values) {
+ public Builder addStringSequenceVariable(String name, Iterable<String> values) {
Preconditions.checkArgument(
!variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
- variablesMap.put(name, new StringSequence(values));
- return this;
- }
-
- /** Add a sequence variable that expands {@code name} to {@code values}. */
- public Builder addStringSequenceVariable(String name, NestedSet<String> values) {
- Preconditions.checkArgument(
- !variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
- variablesMap.put(name, new StringSequence(values));
+ ImmutableList.Builder<String> builder = ImmutableList.builder();
+ builder.addAll(values);
+ variablesMap.put(name, new StringSequence(builder.build()));
return this;
}
@@ -1196,8 +1188,10 @@
for (String name : variables.keySet()) {
Preconditions.checkArgument(
!variablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
- variablesMap.put(name, new StringValue(variables.get(name)));
+ Preconditions.checkArgument(
+ !stringVariablesMap.containsKey(name), "Cannot overwrite variable '%s'", name);
}
+ stringVariablesMap.putAll(variables);
return this;
}
@@ -1205,9 +1199,15 @@
public Builder addAll(Variables variables) {
SetView<String> intersection =
Sets.intersection(variables.variablesMap.keySet(), variablesMap.keySet());
+ SetView<String> stringIntersection =
+ Sets.intersection(variables.stringVariablesMap.keySet(), stringVariablesMap.keySet());
Preconditions.checkArgument(
intersection.isEmpty(), "Cannot overwrite existing variables: %s", intersection);
+ Preconditions.checkArgument(
+ stringIntersection.isEmpty(),
+ "Cannot overwrite existing variables: %s", stringIntersection);
this.variablesMap.putAll(variables.variablesMap);
+ this.stringVariablesMap.putAll(variables.stringVariablesMap);
return this;
}
@@ -1218,6 +1218,7 @@
*/
Builder addAndOverwriteAll(Variables overwrittenVariables) {
this.variablesMap.putAll(overwrittenVariables.variablesMap);
+ this.stringVariablesMap.putAll(overwrittenVariables.stringVariablesMap);
return this;
}
@@ -1225,7 +1226,8 @@
* @return a new {@Variables} object.
*/
Variables build() {
- return new Variables(ImmutableMap.copyOf(variablesMap));
+ return new Variables(
+ ImmutableMap.copyOf(variablesMap), ImmutableMap.copyOf(stringVariablesMap));
}
}
@@ -1238,10 +1240,14 @@
}
private final ImmutableMap<String, VariableValue> variablesMap;
+ private final ImmutableMap<String, String> stringVariablesMap;
private final Variables parent;
- private Variables(ImmutableMap<String, VariableValue> variablesMap) {
+ private Variables(
+ ImmutableMap<String, VariableValue> variablesMap,
+ ImmutableMap<String, String> stringVariablesMap) {
this.variablesMap = variablesMap;
+ this.stringVariablesMap = stringVariablesMap;
this.parent = null;
}
@@ -1251,6 +1257,7 @@
*/
private Variables(Variables parent, String name, VariableValue value) {
this.variablesMap = ImmutableMap.of(name, value);
+ this.stringVariablesMap = ImmutableMap.of();
this.parent = parent;
}
@@ -1302,6 +1309,9 @@
if (variablesMap.containsKey(name)) {
return Pair.of(variablesMap.get(name), null);
}
+ if (stringVariablesMap.containsKey(name)) {
+ return Pair.<VariableValue, String>of(new StringValue(stringVariablesMap.get(name)), null);
+ }
if (parent != null) {
return parent.getNonStructuredVariable(name);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 032624b..a368cbb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -1217,8 +1217,8 @@
return rpathEntries;
}
- public ImmutableList<String> getLibopts() {
- return libopts.asList();
+ public ImmutableSet<String> getLibopts() {
+ return libopts;
}
public ImmutableList<String> getLinkerInputParams() {
@@ -1300,7 +1300,7 @@
}
buildVariables.addStringSequenceVariable(
- LINKSTAMP_PATHS_VARIABLE, linkstampPaths.build().asList());
+ LINKSTAMP_PATHS_VARIABLE, linkstampPaths.build());
// pic
boolean forcePic = cppConfiguration.forcePic();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index d1eadd4..8aa4ecc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -342,19 +342,19 @@
}
/** Get the safe path strings for a list of paths to use in the build variables. */
- private ImmutableList<String> getSafePathStrings(Collection<PathFragment> paths) {
+ private ImmutableSet<String> getSafePathStrings(Collection<PathFragment> paths) {
ImmutableSet.Builder<String> result = ImmutableSet.builder();
for (PathFragment path : paths) {
result.add(path.getSafePathString());
}
- return result.build().asList();
+ return result.build();
}
/**
* Select .pcm inputs to pass on the command line depending on whether we are in pic or non-pic
* mode.
*/
- private ImmutableList<String> getHeaderModulePaths(
+ private ImmutableSet<String> getHeaderModulePaths(
CppCompileActionBuilder builder, boolean usePic) {
ImmutableSet.Builder<String> result = ImmutableSet.builder();
Iterable<Artifact> artifacts =
@@ -364,7 +364,7 @@
for (Artifact artifact : artifacts) {
result.add(artifact.getExecPathString());
}
- return result.build().asList();
+ return result.build();
}
private void setupCompileBuildVariables(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java
index 8d872b0..f8e3802 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcVariablesExtension.java
@@ -170,22 +170,21 @@
FULLY_LINKED_ARCHIVE_PATH_VARIABLE_NAME, fullyLinkArchive.getExecPathString());
builder.addStringSequenceVariable(
OBJC_LIBRARY_EXEC_PATHS_VARIABLE_NAME,
- ImmutableList.copyOf(Artifact.toExecPaths(objcProvider.getObjcLibraries())));
+ Artifact.toExecPaths(objcProvider.getObjcLibraries()));
builder.addStringSequenceVariable(
CC_LIBRARY_EXEC_PATHS_VARIABLE_NAME,
- ImmutableList.copyOf(Artifact.toExecPaths(objcProvider.getCcLibraries())));
+ Artifact.toExecPaths(objcProvider.getCcLibraries()));
builder.addStringSequenceVariable(
IMPORTED_LIBRARY_EXEC_PATHS_VARIABLE_NAME,
- ImmutableList.copyOf(Artifact.toExecPaths(objcProvider.get(IMPORTED_LIBRARY))));
+ Artifact.toExecPaths(objcProvider.get(IMPORTED_LIBRARY)));
}
private void addExecutableLinkVariables(CcToolchainFeatures.Variables.Builder builder) {
builder.addStringSequenceVariable(
- FRAMEWORK_NAMES_VARIABLE_NAME, ImmutableList.copyOf(frameworkNames));
+ FRAMEWORK_NAMES_VARIABLE_NAME, frameworkNames);
builder.addStringSequenceVariable(
WEAK_FRAMEWORK_NAMES_VARIABLE_NAME,
- ImmutableList.copyOf(
- SdkFramework.names(objcProvider.get(ObjcProvider.WEAK_SDK_FRAMEWORK))));
+ SdkFramework.names(objcProvider.get(ObjcProvider.WEAK_SDK_FRAMEWORK)));
builder.addStringSequenceVariable(LIBRARY_NAMES_VARIABLE_NAME, libraryNames);
builder.addStringVariable(
FILELIST_VARIABLE_NAME, intermediateArtifacts.linkerObjList().getExecPathString());
@@ -197,7 +196,7 @@
builder.addStringSequenceVariable(
FORCE_LOAD_EXEC_PATHS_VARIABLE_NAME,
- ImmutableList.copyOf(Artifact.toExecPaths(forceLoadArtifacts)));
+ Artifact.toExecPaths(forceLoadArtifacts));
builder.addStringSequenceVariable(DEP_LINKOPTS_VARIABLE_NAME, objcProvider.get(LINKOPT));
builder.addStringSequenceVariable(ATTR_LINKOPTS_VARIABLE_NAME, attributeLinkopts);
}
