diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 845c6f35242687..8b7af40377a6a8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -562,6 +562,15 @@ private void setCoptsFilter(CoptsFilter coptsFilter) { this.coptsFilter = Preconditions.checkNotNull(coptsFilter); } + /** + * Adds the given defines to the compiler command line of this target as well as its dependent + * targets. + */ + public CcCompilationHelper addDefines(NestedSet defines) { + this.defines.addAll(defines.toList()); + return this; + } + /** * Adds the given defines to the compiler command line of this target as well as its dependent * targets. @@ -595,6 +604,15 @@ private void setLooseIncludeDirs(Set looseIncludeDirs) { this.looseIncludeDirs = looseIncludeDirs; } + /** + * Adds the given directories to the system include directories (they are passed with {@code + * "-isystem"} to the compiler); these are also passed to dependent rules. + */ + public CcCompilationHelper addSystemIncludeDirs(NestedSet systemIncludeDirs) { + this.systemIncludeDirs.addAll(systemIncludeDirs.toList()); + return this; + } + /** * Adds the given directories to the system include directories (they are passed with {@code * "-isystem"} to the compiler); these are also passed to dependent rules. @@ -604,6 +622,15 @@ public CcCompilationHelper addSystemIncludeDirs(Iterable systemInc return this; } + /** + * Adds the given directories to the quote include directories (they are passed with {@code + * "-iquote"} to the compiler); these are also passed to dependent rules. + */ + public CcCompilationHelper addQuoteIncludeDirs(NestedSet quoteIncludeDirs) { + this.quoteIncludeDirs.addAll(quoteIncludeDirs.toList()); + return this; + } + /** * Adds the given directories to the quote include directories (they are passed with {@code * "-iquote"} to the compiler); these are also passed to dependent rules. @@ -613,6 +640,15 @@ public CcCompilationHelper addQuoteIncludeDirs(Iterable quoteInclu return this; } + /** + * Adds the given directories to the include directories (they are passed with {@code "-I"} to the + * compiler); these are also passed to dependent rules. + */ + public CcCompilationHelper addIncludeDirs(NestedSet includeDirs) { + this.includeDirs.addAll(includeDirs.toList()); + return this; + } + /** * Adds the given directories to the include directories (they are passed with {@code "-I"} to the * compiler); these are also passed to dependent rules. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java index 5035d791d8c71b..3746a1c2779ac7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java @@ -965,6 +965,66 @@ public int hashCode() { } } + /** + * A sequence of simple string values. Exists as a memory optimization - a typical build can + * contain millions of feature values, so getting rid of the overhead of {@code StringValue} + * objects significantly reduces memory overhead. + */ + @Immutable + @AutoCodec + static final class StringSetSequence extends VariableValueAdapter { + private final NestedSet values; + private int hash = 0; + + public StringSetSequence(NestedSet values) { + Preconditions.checkNotNull(values); + this.values = values; + } + + @Override + public Iterable getSequenceValue(String variableName) { + final ImmutableList.Builder sequences = ImmutableList.builder(); + for (String value : values.toList()) { + sequences.add(new StringValue(value)); + } + return sequences.build(); + } + + @Override + public String getVariableTypeName() { + return Sequence.SEQUENCE_VARIABLE_TYPE_NAME; + } + + @Override + public boolean isTruthy() { + return !values.isEmpty(); + } + + @Override + public boolean equals(Object other) { + if (!(other instanceof StringSetSequence)) { + return false; + } + if (this == other) { + return true; + } + return values.equals(((StringSetSequence) other).values); + } + + @Override + public int hashCode() { + int h = hash; + if (h == 0) { + h = 1; + for (String s : values.toList()) { + h = 31 * h + (s == null ? 0 : s.hashCode()); + } + hash = h; + } + return h; + } + } + /** * Single structure value. Be careful not to create sequences of single structures, as the memory * overhead is prohibitively big. Use optimized {@link StructureSequence} instead. @@ -1177,7 +1237,7 @@ public Builder addStringSequenceVariable(String name, ImmutableSet value public Builder addStringSequenceVariable(String name, NestedSet values) { checkVariableNotPresentAlready(name); Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name); - variablesMap.put(name, new StringSequence(values)); + variablesMap.put(name, new StringSetSequence(values)); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java index 4c21beee4f6453..5c6ee83ccdfa1c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java @@ -360,10 +360,10 @@ public static void setupCommonVariables( List variablesExtensions, Map additionalBuildVariables, Iterable directModuleMaps, - Iterable includeDirs, - Iterable quoteIncludeDirs, - Iterable systemIncludeDirs, - Iterable frameworkIncludeDirs, + List includeDirs, + List quoteIncludeDirs, + List systemIncludeDirs, + List frameworkIncludeDirs, NestedSet defines, Iterable localDefines) { setupCommonVariablesInternal( @@ -461,7 +461,12 @@ private static void setupCommonVariablesInternal( } /** Get the safe path strings for a list of paths to use in the build variables. */ - private static NestedSet getSafePathStrings(Iterable paths) { + private static NestedSet getSafePathStrings(NestedSet paths) { + return getSafePathStrings(paths.toList()); + } + + /** Get the safe path strings for a list of paths to use in the build variables. */ + private static NestedSet getSafePathStrings(List paths) { // Using ImmutableSet first to remove duplicates, then NestedSet for smaller memory footprint. return NestedSetBuilder.wrap( Order.STABLE_ORDER, 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 df797e8c6ab609..cbdce17a5cc255 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 @@ -859,7 +859,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException { NestedSet expandedLinkerArtifacts = getArtifactsPossiblyLtoMapped( - collectedLibrariesToLink.getExpandedLinkerInputs(), ltoMapping); + collectedLibrariesToLink.getExpandedLinkerInputs().toList(), ltoMapping); CcToolchainVariables variables; try { @@ -1065,7 +1065,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException { inputsBuilder.addTransitive(linkstampObjectArtifacts); ImmutableSet fakeLinkerInputArtifacts = - collectedLibrariesToLink.getExpandedLinkerInputs().stream() + collectedLibrariesToLink.getExpandedLinkerInputs().toList().stream() .filter(LinkerInput::isFake) .map(LinkerInput::getArtifact) .collect(ImmutableSet.toImmutableSet()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java index 7cc101d36b0061..b76a65ef5396c4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibrariesToLinkCollector.java @@ -19,6 +19,8 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.LibraryToLinkValue; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.SequenceBuilder; @@ -115,15 +117,15 @@ public LibrariesToLinkCollector( */ public static class CollectedLibrariesToLink { private final SequenceBuilder librariesToLink; - private final ImmutableSet expandedLinkerInputs; - private final ImmutableSet librarySearchDirectories; - private final ImmutableSet runtimeLibrarySearchDirectories; + private final NestedSet expandedLinkerInputs; + private final NestedSet librarySearchDirectories; + private final NestedSet runtimeLibrarySearchDirectories; private CollectedLibrariesToLink( SequenceBuilder librariesToLink, - ImmutableSet expandedLinkerInputs, - ImmutableSet librarySearchDirectories, - ImmutableSet runtimeLibrarySearchDirectories) { + NestedSet expandedLinkerInputs, + NestedSet librarySearchDirectories, + NestedSet runtimeLibrarySearchDirectories) { this.librariesToLink = librariesToLink; this.expandedLinkerInputs = expandedLinkerInputs; this.librarySearchDirectories = librarySearchDirectories; @@ -135,15 +137,15 @@ public SequenceBuilder getLibrariesToLink() { } // TODO(b/78347840): Figure out how to make these Artifacts. - public ImmutableSet getExpandedLinkerInputs() { + public NestedSet getExpandedLinkerInputs() { return expandedLinkerInputs; } - public ImmutableSet getLibrarySearchDirectories() { + public NestedSet getLibrarySearchDirectories() { return librarySearchDirectories; } - public ImmutableSet getRuntimeLibrarySearchDirectories() { + public NestedSet getRuntimeLibrarySearchDirectories() { return runtimeLibrarySearchDirectories; } } @@ -158,10 +160,10 @@ public ImmutableSet getRuntimeLibrarySearchDirectories() { *

TODO: Factor out of the bazel binary into build variables for crosstool action_configs. */ public CollectedLibrariesToLink collectLibrariesToLink() { - ImmutableSet.Builder librarySearchDirectories = ImmutableSet.builder(); - ImmutableSet.Builder runtimeLibrarySearchDirectories = ImmutableSet.builder(); + NestedSetBuilder librarySearchDirectories = NestedSetBuilder.linkOrder(); + NestedSetBuilder runtimeLibrarySearchDirectories = NestedSetBuilder.linkOrder(); ImmutableSet.Builder rpathRootsForExplicitSoDeps = ImmutableSet.builder(); - ImmutableSet.Builder expandedLinkerInputsBuilder = ImmutableSet.builder(); + NestedSetBuilder expandedLinkerInputsBuilder = NestedSetBuilder.linkOrder(); // List of command line parameters that need to be placed *outside* of // --whole-archive ... --no-whole-archive. SequenceBuilder librariesToLink = new SequenceBuilder(); @@ -210,14 +212,14 @@ public CollectedLibrariesToLink collectLibrariesToLink() { Preconditions.checkState( ltoMap == null || ltoMap.isEmpty(), "Still have LTO objects left: %s", ltoMap); - ImmutableSet.Builder allRuntimeLibrarySearchDirectories = ImmutableSet.builder(); + NestedSetBuilder allRuntimeLibrarySearchDirectories = NestedSetBuilder.linkOrder(); // rpath ordering matters for performance; first add the one where most libraries are found. if (includeSolibDir) { allRuntimeLibrarySearchDirectories.add(rpathRoot); } allRuntimeLibrarySearchDirectories.addAll(rpathRootsForExplicitSoDeps.build()); if (includeToolchainLibrariesSolibDir) { - allRuntimeLibrarySearchDirectories.addAll(runtimeLibrarySearchDirectories.build()); + allRuntimeLibrarySearchDirectories.addTransitive(runtimeLibrarySearchDirectories.build()); } return new CollectedLibrariesToLink( @@ -228,10 +230,10 @@ public CollectedLibrariesToLink collectLibrariesToLink() { } private Pair addLinkerInputs( - ImmutableSet.Builder librarySearchDirectories, + NestedSetBuilder librarySearchDirectories, ImmutableSet.Builder rpathEntries, SequenceBuilder librariesToLink, - ImmutableSet.Builder expandedLinkerInputsBuilder) { + NestedSetBuilder expandedLinkerInputsBuilder) { boolean includeSolibDir = false; boolean includeToolchainLibrariesSolibDir = false; for (LinkerInput input : linkerInputs) { @@ -276,8 +278,8 @@ private Pair addLinkerInputs( private void addDynamicInputLinkOptions( LinkerInput input, SequenceBuilder librariesToLink, - ImmutableSet.Builder expandedLinkerInputsBuilder, - ImmutableSet.Builder librarySearchDirectories, + NestedSetBuilder expandedLinkerInputsBuilder, + NestedSetBuilder librarySearchDirectories, ImmutableSet.Builder rpathRootsForExplicitSoDeps) { Preconditions.checkState( input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY @@ -359,7 +361,7 @@ private static String effectiveObjectFilePath(Artifact objectFile, boolean input private void addStaticInputLinkOptions( LinkerInput input, SequenceBuilder librariesToLink, - ImmutableSet.Builder expandedLinkerInputsBuilder) { + NestedSetBuilder expandedLinkerInputsBuilder) { ArtifactCategory artifactCategory = input.getArtifactCategory(); Preconditions.checkArgument( artifactCategory.equals(ArtifactCategory.OBJECT_FILE) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java index 7fc4def8b970f6..68a67c4fcfe071 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.SequenceBuilder; import com.google.devtools.build.lib.syntax.EvalException; @@ -112,9 +113,9 @@ public static CcToolchainVariables setupVariables( PathFragment ltoOutputRootPrefix, String defFile, FdoContext fdoContext, - Iterable runtimeLibrarySearchDirectories, + NestedSet runtimeLibrarySearchDirectories, SequenceBuilder librariesToLink, - Iterable librarySearchDirectories, + NestedSet librarySearchDirectories, boolean addIfsoRelatedVariables) throws EvalException { CcToolchainVariables.Builder buildVariables =