Reduce memory cost of `ObjcLibrary`.
`LibraryToLink`'s generated `AutoValue` class contains 20 fields, but `ObjcLibrary` typically only needs two of them - the rest are either `null` or `false`. To accommodate this, introduced a specialized `StaticOnlyLibraryToLink` implementation with only the two necessary fields.
We lose out on the memoization of the four `LinkerInputs.LibraryToLink`-returning methods, but for `ObjcLibrary` these are never called multiple times anyway. It seems rare that they are ever called multiple times, but I left the memoization (using `@Memoized` now) for the non-static-only case.
To reduce duplicate instances, intern based on the defining `Artifact`. This also takes care of duplicate strings created in `CcLinkerOutputs#libraryIdentifierOf`.
Made some other tangentially related optimizations.
PiperOrigin-RevId: 361654318
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java
index cc9bece..9ea6bad 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java
@@ -37,6 +37,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
+import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Sequence;
@@ -193,7 +194,7 @@
private final ImmutableList<Artifact> nonCodeInputs;
private final ImmutableList<Linkstamp> linkstamps;
- private LinkerInput(
+ public LinkerInput(
Label owner,
ImmutableList<LibraryToLink> libraries,
ImmutableList<LinkOptions> userLinkFlags,
@@ -369,7 +370,7 @@
}
private final NestedSet<LinkerInput> linkerInputs;
- private final ExtraLinkTimeLibraries extraLinkTimeLibraries;
+ @Nullable private final ExtraLinkTimeLibraries extraLinkTimeLibraries;
@Override
public void debugPrint(Printer printer) {
@@ -382,7 +383,8 @@
}
public CcLinkingContext(
- NestedSet<LinkerInput> linkerInputs, ExtraLinkTimeLibraries extraLinkTimeLibraries) {
+ NestedSet<LinkerInput> linkerInputs,
+ @Nullable ExtraLinkTimeLibraries extraLinkTimeLibraries) {
this.linkerInputs = linkerInputs;
this.extraLinkTimeLibraries = extraLinkTimeLibraries;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingOutputs.java
index 4c7d300..80344e6 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingOutputs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingOutputs.java
@@ -84,21 +84,21 @@
return result.build();
}
+ private static final ImmutableList<String> PIC_SUFFIXES =
+ ImmutableList.of(".pic.a", ".nopic.a", ".pic.lo");
+
/**
* Returns the library identifier of an artifact: a string that is different for different
* libraries, but is the same for the shared, static and pic versions of the same library.
*/
public static String libraryIdentifierOf(Artifact libraryArtifact) {
String name = libraryArtifact.getRootRelativePath().getPathString();
- String basename = FileSystemUtils.removeExtension(name);
- // Need to special-case file types with double extension.
- return name.endsWith(".pic.a")
- ? FileSystemUtils.removeExtension(basename)
- : name.endsWith(".nopic.a")
- ? FileSystemUtils.removeExtension(basename)
- : name.endsWith(".pic.lo")
- ? FileSystemUtils.removeExtension(basename)
- : basename;
+ for (String picSuffix : PIC_SUFFIXES) {
+ if (name.endsWith(picSuffix)) {
+ return name.substring(0, name.length() - picSuffix.length());
+ }
+ }
+ return FileSystemUtils.removeExtension(name);
}
public static Builder builder() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java
index 920caf3..2316ba2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java
@@ -1,4 +1,3 @@
-package com.google.devtools.build.lib.rules.cpp;
// Copyright 2018 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,18 +12,24 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+package com.google.devtools.build.lib.rules.cpp;
+
import com.google.auto.value.AutoValue;
+import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.starlarkbuildapi.cpp.LibraryToLinkApi;
-import java.util.List;
import javax.annotation.Nullable;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
@@ -33,26 +38,72 @@
import net.starlark.java.eval.StarlarkList;
import net.starlark.java.eval.StarlarkThread;
-/**
- * Encapsulates information for linking a library.
- *
- * <p>TODO(b/118663806): This class which shall be renamed later to LibraryToLink (once the old
- * LibraryToLink implementation is removed) will have all the information necessary for linking a
- * library in all of its variants : static params for executable, static params for dynamic library,
- * dynamic params for executable and dynamic params for dynamic library.
- */
-@AutoValue
+/** Encapsulates information for linking a library. */
+// The AutoValue implementation of this class already has a sizeable number of fields, meaning that
+// instances have a surprising memory cost. We may benefit from having more specialized
+// implementations similar to StaticOnlyLibraryToLink, for cases when certain fields are always
+// null. Consider this before adding additional fields to this class. See b/181991741.
@Immutable
public abstract class LibraryToLink implements LibraryToLinkApi<Artifact, LtoBackendArtifacts> {
+ public static final Depset.ElementType TYPE = Depset.ElementType.of(LibraryToLink.class);
+
+ public static ImmutableList<Artifact> getDynamicLibrariesForRuntime(
+ boolean linkingStatically, Iterable<LibraryToLink> libraries) {
+ ImmutableList.Builder<Artifact> dynamicLibrariesForRuntimeBuilder = ImmutableList.builder();
+ for (LibraryToLink libraryToLink : libraries) {
+ Artifact artifact = libraryToLink.getDynamicLibraryForRuntimeOrNull(linkingStatically);
+ if (artifact != null) {
+ dynamicLibrariesForRuntimeBuilder.add(artifact);
+ }
+ }
+ return dynamicLibrariesForRuntimeBuilder.build();
+ }
+
+ public static ImmutableList<Artifact> getDynamicLibrariesForLinking(
+ NestedSet<LibraryToLink> libraries) {
+ ImmutableList.Builder<Artifact> dynamicLibrariesForLinkingBuilder = ImmutableList.builder();
+ for (LibraryToLink libraryToLink : libraries.toList()) {
+ if (libraryToLink.getInterfaceLibrary() != null) {
+ dynamicLibrariesForLinkingBuilder.add(libraryToLink.getInterfaceLibrary());
+ } else if (libraryToLink.getDynamicLibrary() != null) {
+ dynamicLibrariesForLinkingBuilder.add(libraryToLink.getDynamicLibrary());
+ }
+ }
+ return dynamicLibrariesForLinkingBuilder.build();
+ }
+
+ private LibraryToLink() {}
+
+ public abstract String getLibraryIdentifier();
+
+ @Nullable
+ public abstract ImmutableList<Artifact> getObjectFiles();
+
+ @Nullable
+ public abstract ImmutableMap<Artifact, LtoBackendArtifacts> getSharedNonLtoBackends();
+
+ @Nullable
+ public abstract LtoCompilationContext getLtoCompilationContext();
+
+ @Nullable
+ public abstract ImmutableList<Artifact> getPicObjectFiles();
+
+ @Nullable
+ public abstract ImmutableMap<Artifact, LtoBackendArtifacts> getPicSharedNonLtoBackends();
+
+ @Nullable
+ public abstract LtoCompilationContext getPicLtoCompilationContext();
+
+ public abstract AutoLibraryToLink.Builder toBuilder();
+
@Override
- public boolean isImmutable() {
+ public final boolean isImmutable() {
return true; // immutable and Starlark-hashable
}
- public static final Depset.ElementType TYPE = Depset.ElementType.of(LibraryToLink.class);
-
- public Artifact getDynamicLibraryForRuntimeOrNull(boolean linkingStatically) {
+ @Nullable
+ public final Artifact getDynamicLibraryForRuntimeOrNull(boolean linkingStatically) {
if (getDynamicLibrary() == null) {
return null;
}
@@ -62,115 +113,111 @@
return getDynamicLibrary();
}
- private LinkerInputs.LibraryToLink picStaticLibraryToLink;
- private LinkerInputs.LibraryToLink staticLibraryToLink;
- private LinkerInputs.LibraryToLink dynamicLibraryToLink;
- private LinkerInputs.LibraryToLink interfaceLibraryToLink;
-
- public abstract String getLibraryIdentifier();
-
- @Nullable
@Override
- public abstract Artifact getStaticLibrary();
-
- @Nullable
- public abstract ImmutableList<Artifact> getObjectFiles();
-
- @Nullable
- @Override
- public Sequence<Artifact> getObjectFilesForStarlark() {
- if (getObjectFiles() == null) {
- return StarlarkList.empty();
- }
- return StarlarkList.immutableCopyOf(getObjectFiles());
+ public final Sequence<Artifact> getObjectFilesForStarlark() {
+ ImmutableList<Artifact> objectFiles = getObjectFiles();
+ return objectFiles == null ? StarlarkList.empty() : StarlarkList.immutableCopyOf(objectFiles);
}
@Override
- public boolean getMustKeepDebugForStarlark(StarlarkThread thread) throws EvalException {
+ public final Sequence<Artifact> getLtoBitcodeFilesForStarlark() {
+ LtoCompilationContext ctx = getLtoCompilationContext();
+ return ctx == null ? StarlarkList.empty() : StarlarkList.immutableCopyOf(ctx.getBitcodeFiles());
+ }
+
+ @Override
+ public final boolean getMustKeepDebugForStarlark(StarlarkThread thread) throws EvalException {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
return getMustKeepDebug();
}
- @Nullable
@Override
- public Sequence<Artifact> getLtoBitcodeFilesForStarlark() {
- if (getLtoCompilationContext() == null) {
- return StarlarkList.empty();
- }
- return StarlarkList.immutableCopyOf(getLtoCompilationContext().getBitcodeFiles());
- }
-
- @Nullable
- public abstract ImmutableMap<Artifact, LtoBackendArtifacts> getSharedNonLtoBackends();
-
- @Nullable
- @Override
- public Dict<Artifact, LtoBackendArtifacts> getSharedNonLtoBackendsForStarlark(
+ public final Dict<Artifact, LtoBackendArtifacts> getSharedNonLtoBackendsForStarlark(
StarlarkThread thread) throws EvalException {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
return Dict.immutableCopyOf(getSharedNonLtoBackends());
}
- @Nullable
- public abstract LtoCompilationContext getLtoCompilationContext();
-
- @Nullable
@Override
- public abstract Artifact getPicStaticLibrary();
-
- @Nullable
- public abstract ImmutableList<Artifact> getPicObjectFiles();
-
- @Nullable
- @Override
- public Sequence<Artifact> getPicObjectFilesForStarlark() {
- if (getPicObjectFiles() == null) {
- return StarlarkList.empty();
- }
- return StarlarkList.immutableCopyOf(getPicObjectFiles());
+ public final Sequence<Artifact> getPicObjectFilesForStarlark() {
+ ImmutableList<Artifact> objectFiles = getPicObjectFiles();
+ return objectFiles == null ? StarlarkList.empty() : StarlarkList.immutableCopyOf(objectFiles);
}
- @Nullable
@Override
- public Sequence<Artifact> getPicLtoBitcodeFilesForStarlark() {
- if (getPicLtoCompilationContext() == null) {
- return StarlarkList.empty();
- }
- return StarlarkList.immutableCopyOf(getPicLtoCompilationContext().getBitcodeFiles());
+ public final Sequence<Artifact> getPicLtoBitcodeFilesForStarlark() {
+ LtoCompilationContext ctx = getPicLtoCompilationContext();
+ return ctx == null ? StarlarkList.empty() : StarlarkList.immutableCopyOf(ctx.getBitcodeFiles());
}
- @Nullable
- public abstract ImmutableMap<Artifact, LtoBackendArtifacts> getPicSharedNonLtoBackends();
-
- @Nullable
@Override
- public Dict<Artifact, LtoBackendArtifacts> getPicSharedNonLtoBackendsForStarlark(
+ public final Dict<Artifact, LtoBackendArtifacts> getPicSharedNonLtoBackendsForStarlark(
StarlarkThread thread) throws EvalException {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
return Dict.immutableCopyOf(getPicSharedNonLtoBackends());
}
- @Nullable
- public abstract LtoCompilationContext getPicLtoCompilationContext();
+ LinkerInputs.LibraryToLink getStaticLibraryToLink() {
+ return LinkerInputs.newInputLibrary(
+ Preconditions.checkNotNull(getStaticLibrary(), this),
+ getAlwayslink()
+ ? ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY
+ : ArtifactCategory.STATIC_LIBRARY,
+ getLibraryIdentifier(),
+ getObjectFiles(),
+ getLtoCompilationContext(),
+ getSharedNonLtoBackends(),
+ getMustKeepDebug(),
+ getDisableWholeArchive());
+ }
- @Nullable
- @Override
- public abstract Artifact getDynamicLibrary();
+ LinkerInputs.LibraryToLink getPicStaticLibraryToLink() {
+ return LinkerInputs.newInputLibrary(
+ Preconditions.checkNotNull(getPicStaticLibrary(), this),
+ getAlwayslink()
+ ? ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY
+ : ArtifactCategory.STATIC_LIBRARY,
+ getLibraryIdentifier(),
+ getPicObjectFiles(),
+ getPicLtoCompilationContext(),
+ getPicSharedNonLtoBackends(),
+ getMustKeepDebug(),
+ getDisableWholeArchive());
+ }
- @Nullable
- @Override
- public abstract Artifact getResolvedSymlinkDynamicLibrary();
+ LinkerInputs.LibraryToLink getDynamicLibraryToLink() {
+ Artifact dynamicLibrary = Preconditions.checkNotNull(getDynamicLibrary(), this);
+ if (getResolvedSymlinkDynamicLibrary() != null) {
+ return LinkerInputs.solibLibraryToLink(
+ dynamicLibrary, getResolvedSymlinkDynamicLibrary(), getLibraryIdentifier());
+ }
+ return LinkerInputs.newInputLibrary(
+ dynamicLibrary,
+ ArtifactCategory.DYNAMIC_LIBRARY,
+ getLibraryIdentifier(),
+ /*objectFiles=*/ ImmutableSet.of(),
+ LtoCompilationContext.EMPTY,
+ /*sharedNonLtoBackends=*/ ImmutableMap.of(),
+ getMustKeepDebug(),
+ getDisableWholeArchive());
+ }
- @Nullable
- @Override
- public abstract Artifact getInterfaceLibrary();
-
- @Nullable
- @Override
- public abstract Artifact getResolvedSymlinkInterfaceLibrary();
-
- @Override
- public abstract boolean getAlwayslink();
+ LinkerInputs.LibraryToLink getInterfaceLibraryToLink() {
+ Artifact interfaceLibrary = Preconditions.checkNotNull(getInterfaceLibrary(), this);
+ if (getResolvedSymlinkInterfaceLibrary() != null) {
+ return LinkerInputs.solibLibraryToLink(
+ interfaceLibrary, getResolvedSymlinkInterfaceLibrary(), getLibraryIdentifier());
+ }
+ return LinkerInputs.newInputLibrary(
+ interfaceLibrary,
+ ArtifactCategory.INTERFACE_LIBRARY,
+ getLibraryIdentifier(),
+ /*objectFiles=*/ ImmutableSet.of(),
+ LtoCompilationContext.EMPTY,
+ /*sharedNonLtoBackends=*/ ImmutableMap.of(),
+ getMustKeepDebug(),
+ getDisableWholeArchive());
+ }
// TODO(plf): This is just needed for Go, do not expose to Starlark and try to remove it. This was
// introduced to let a linker input declare that it needs debug info in the executable.
@@ -181,7 +228,7 @@
abstract boolean getDisableWholeArchive();
@Override
- public void debugPrint(Printer printer) {
+ public final void debugPrint(Printer printer) {
printer.append("<LibraryToLink(");
printer.append(
Joiner.on(", ")
@@ -200,228 +247,297 @@
printer.append(")>");
}
- private static String mapEntry(String keyName, Object value) {
- if (value == null) {
- return null;
- } else {
- return keyName + "=" + value;
- }
+ private static String mapEntry(String keyName, @Nullable Object value) {
+ return value == null ? null : keyName + "=" + value;
}
- public static Builder builder() {
- return new AutoValue_LibraryToLink.Builder()
+ public static AutoLibraryToLink.Builder builder() {
+ return new AutoValue_LibraryToLink_AutoLibraryToLink.Builder()
.setMustKeepDebug(false)
.setAlwayslink(false)
.setDisableWholeArchive(false);
}
- LinkerInputs.LibraryToLink getStaticLibraryToLink() {
- Preconditions.checkNotNull(getStaticLibrary(), this);
- if (staticLibraryToLink != null) {
- return staticLibraryToLink;
- }
- staticLibraryToLink =
- LinkerInputs.newInputLibrary(
- getStaticLibrary(),
- getAlwayslink()
- ? ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY
- : ArtifactCategory.STATIC_LIBRARY,
- getLibraryIdentifier(),
- getObjectFiles(),
- getLtoCompilationContext(),
- getSharedNonLtoBackends(),
- getMustKeepDebug(),
- getDisableWholeArchive());
- return staticLibraryToLink;
+ /**
+ * Creates a {@link LibraryToLink} that has only {@link #getStaticLibrary} and no other optional
+ * fields.
+ */
+ public static LibraryToLink staticOnly(Artifact staticLibrary) {
+ return StaticOnlyLibraryToLink.cache.getUnchecked(staticLibrary);
}
- LinkerInputs.LibraryToLink getPicStaticLibraryToLink() {
- Preconditions.checkNotNull(getPicStaticLibrary(), this);
- if (picStaticLibraryToLink != null) {
- return picStaticLibraryToLink;
- }
- picStaticLibraryToLink =
- LinkerInputs.newInputLibrary(
- getPicStaticLibrary(),
- getAlwayslink()
- ? ArtifactCategory.ALWAYSLINK_STATIC_LIBRARY
- : ArtifactCategory.STATIC_LIBRARY,
- getLibraryIdentifier(),
- getPicObjectFiles(),
- getPicLtoCompilationContext(),
- getPicSharedNonLtoBackends(),
- getMustKeepDebug(),
- getDisableWholeArchive());
- return picStaticLibraryToLink;
- }
+ /** Builder for {@link LibraryToLink}. */
+ public interface Builder {
- LinkerInputs.LibraryToLink getDynamicLibraryToLink() {
- Preconditions.checkNotNull(getDynamicLibrary(), this);
- if (dynamicLibraryToLink != null) {
- return dynamicLibraryToLink;
- }
- if (getResolvedSymlinkDynamicLibrary() != null) {
- dynamicLibraryToLink =
- LinkerInputs.solibLibraryToLink(
- getDynamicLibrary(), getResolvedSymlinkDynamicLibrary(), getLibraryIdentifier());
- } else {
- dynamicLibraryToLink =
- LinkerInputs.newInputLibrary(
- getDynamicLibrary(),
- ArtifactCategory.DYNAMIC_LIBRARY,
- getLibraryIdentifier(),
- /* objectFiles */ ImmutableSet.of(),
- LtoCompilationContext.EMPTY,
- /* sharedNonLtoBackends */ ImmutableMap.of(),
- getMustKeepDebug(),
- getDisableWholeArchive());
- }
- return dynamicLibraryToLink;
- }
+ AutoLibraryToLink.Builder setLibraryIdentifier(String libraryIdentifier);
- LinkerInputs.LibraryToLink getInterfaceLibraryToLink() {
- Preconditions.checkNotNull(getInterfaceLibrary());
- if (interfaceLibraryToLink != null) {
- return interfaceLibraryToLink;
- }
- if (getResolvedSymlinkInterfaceLibrary() != null) {
- interfaceLibraryToLink =
- LinkerInputs.solibLibraryToLink(
- getInterfaceLibrary(), getResolvedSymlinkInterfaceLibrary(), getLibraryIdentifier());
- } else {
- interfaceLibraryToLink =
- LinkerInputs.newInputLibrary(
- getInterfaceLibrary(),
- ArtifactCategory.INTERFACE_LIBRARY,
- getLibraryIdentifier(),
- /* objectFiles */ ImmutableSet.of(),
- LtoCompilationContext.EMPTY,
- /* sharedNonLtoBackends */ ImmutableMap.of(),
- getMustKeepDebug(),
- getDisableWholeArchive());
- }
- return interfaceLibraryToLink;
- }
+ AutoLibraryToLink.Builder setStaticLibrary(Artifact staticLibrary);
- public static List<Artifact> getDynamicLibrariesForRuntime(
- boolean linkingStatically, Iterable<LibraryToLink> libraries) {
- ImmutableList.Builder<Artifact> dynamicLibrariesForRuntimeBuilder = ImmutableList.builder();
- for (LibraryToLink libraryToLink : libraries) {
- Artifact artifact = libraryToLink.getDynamicLibraryForRuntimeOrNull(linkingStatically);
- if (artifact != null) {
- dynamicLibrariesForRuntimeBuilder.add(artifact);
- }
- }
- return dynamicLibrariesForRuntimeBuilder.build();
- }
+ AutoLibraryToLink.Builder setObjectFiles(ImmutableList<Artifact> objectFiles);
- public static List<Artifact> getDynamicLibrariesForLinking(NestedSet<LibraryToLink> libraries) {
- ImmutableList.Builder<Artifact> dynamicLibrariesForLinkingBuilder = ImmutableList.builder();
- for (LibraryToLink libraryToLink : libraries.toList()) {
- if (libraryToLink.getInterfaceLibrary() != null) {
- dynamicLibrariesForLinkingBuilder.add(libraryToLink.getInterfaceLibrary());
- } else if (libraryToLink.getDynamicLibrary() != null) {
- dynamicLibrariesForLinkingBuilder.add(libraryToLink.getDynamicLibrary());
- }
- }
- return dynamicLibrariesForLinkingBuilder.build();
- }
+ AutoLibraryToLink.Builder setLtoCompilationContext(LtoCompilationContext ltoCompilationContext);
- public abstract Builder toBuilder();
-
- /** Builder for LibraryToLink. */
- @AutoValue.Builder
- public abstract static class Builder {
-
- public abstract Builder setLibraryIdentifier(String libraryIdentifier);
-
- public abstract Builder setStaticLibrary(Artifact staticLibrary);
-
- public abstract Builder setObjectFiles(ImmutableList<Artifact> objectFiles);
-
- abstract Builder setLtoCompilationContext(LtoCompilationContext ltoCompilationContext);
-
- abstract Builder setSharedNonLtoBackends(
+ AutoLibraryToLink.Builder setSharedNonLtoBackends(
ImmutableMap<Artifact, LtoBackendArtifacts> sharedNonLtoBackends);
- abstract Builder setPicStaticLibrary(Artifact picStaticLibrary);
+ AutoLibraryToLink.Builder setPicStaticLibrary(Artifact picStaticLibrary);
- abstract Builder setPicObjectFiles(ImmutableList<Artifact> picObjectFiles);
+ AutoLibraryToLink.Builder setPicObjectFiles(ImmutableList<Artifact> picObjectFiles);
- abstract Builder setPicLtoCompilationContext(LtoCompilationContext picLtoCompilationContext);
+ AutoLibraryToLink.Builder setPicLtoCompilationContext(
+ LtoCompilationContext picLtoCompilationContext);
- abstract Builder setPicSharedNonLtoBackends(
+ AutoLibraryToLink.Builder setPicSharedNonLtoBackends(
ImmutableMap<Artifact, LtoBackendArtifacts> picSharedNonLtoBackends);
- public abstract Builder setDynamicLibrary(Artifact dynamicLibrary);
+ AutoLibraryToLink.Builder setDynamicLibrary(Artifact dynamicLibrary);
- public abstract Builder setResolvedSymlinkDynamicLibrary(
+ AutoLibraryToLink.Builder setResolvedSymlinkDynamicLibrary(
Artifact resolvedSymlinkDynamicLibrary);
- public abstract Builder setInterfaceLibrary(Artifact interfaceLibrary);
+ AutoLibraryToLink.Builder setInterfaceLibrary(Artifact interfaceLibrary);
- public abstract Builder setResolvedSymlinkInterfaceLibrary(
+ AutoLibraryToLink.Builder setResolvedSymlinkInterfaceLibrary(
Artifact resolvedSymlinkInterfaceLibrary);
- public abstract Builder setAlwayslink(boolean alwayslink);
+ AutoLibraryToLink.Builder setAlwayslink(boolean alwayslink);
- public abstract Builder setMustKeepDebug(boolean mustKeepDebug);
+ AutoLibraryToLink.Builder setMustKeepDebug(boolean mustKeepDebug);
- public abstract Builder setDisableWholeArchive(boolean disableWholeArchive);
+ AutoLibraryToLink.Builder setDisableWholeArchive(boolean disableWholeArchive);
- // Methods just for validation, not to be called externally.
- abstract LibraryToLink autoBuild();
+ LibraryToLink build();
+ }
- abstract String getLibraryIdentifier();
+ /** {@link AutoValue}-backed implementation. */
+ @AutoValue
+ abstract static class AutoLibraryToLink extends LibraryToLink {
- abstract Artifact getStaticLibrary();
+ @Nullable
+ @Override // Remove @StarlarkMethod.
+ public abstract Artifact getStaticLibrary();
- abstract ImmutableList<Artifact> getObjectFiles();
+ @Nullable
+ @Override // Remove @StarlarkMethod.
+ public abstract Artifact getPicStaticLibrary();
- abstract ImmutableMap<Artifact, LtoBackendArtifacts> getSharedNonLtoBackends();
+ @Nullable
+ @Override // Remove @StarlarkMethod.
+ public abstract Artifact getDynamicLibrary();
- abstract LtoCompilationContext getLtoCompilationContext();
+ @Nullable
+ @Override // Remove @StarlarkMethod.
+ public abstract Artifact getResolvedSymlinkDynamicLibrary();
- abstract Artifact getPicStaticLibrary();
+ @Nullable
+ @Override // Remove @StarlarkMethod.
+ public abstract Artifact getInterfaceLibrary();
- abstract ImmutableList<Artifact> getPicObjectFiles();
+ @Nullable
+ @Override // Remove @StarlarkMethod.
+ public abstract Artifact getResolvedSymlinkInterfaceLibrary();
- abstract ImmutableMap<Artifact, LtoBackendArtifacts> getPicSharedNonLtoBackends();
+ @Override // Remove @StarlarkMethod.
+ public abstract boolean getAlwayslink();
- abstract LtoCompilationContext getPicLtoCompilationContext();
+ @Memoized
+ @Override
+ LinkerInputs.LibraryToLink getStaticLibraryToLink() {
+ return super.getStaticLibraryToLink();
+ }
- abstract Artifact getDynamicLibrary();
+ @Memoized
+ @Override
+ LinkerInputs.LibraryToLink getPicStaticLibraryToLink() {
+ return super.getPicStaticLibraryToLink();
+ }
- abstract Artifact getResolvedSymlinkDynamicLibrary();
+ @Memoized
+ @Override
+ LinkerInputs.LibraryToLink getDynamicLibraryToLink() {
+ return super.getDynamicLibraryToLink();
+ }
- abstract Artifact getInterfaceLibrary();
+ @Memoized
+ @Override
+ LinkerInputs.LibraryToLink getInterfaceLibraryToLink() {
+ return super.getInterfaceLibraryToLink();
+ }
- abstract Artifact getResolvedSymlinkInterfaceLibrary();
+ @AutoValue.Builder
+ public abstract static class Builder implements LibraryToLink.Builder {
- public LibraryToLink build() {
- Preconditions.checkNotNull(getLibraryIdentifier());
- Preconditions.checkState(
- (getObjectFiles() == null
- && getLtoCompilationContext() == null
- && getSharedNonLtoBackends() == null)
- || getStaticLibrary() != null);
- Preconditions.checkState(
- (getPicObjectFiles() == null
- && getPicLtoCompilationContext() == null
- && getPicSharedNonLtoBackends() == null)
- || getPicStaticLibrary() != null);
- Preconditions.checkState(
- getResolvedSymlinkDynamicLibrary() == null || getDynamicLibrary() != null);
- Preconditions.checkState(
- getResolvedSymlinkInterfaceLibrary() == null
- || getResolvedSymlinkInterfaceLibrary() != null);
- Preconditions.checkState(
- getStaticLibrary() != null
- || getPicStaticLibrary() != null
- || getDynamicLibrary() != null
- || getInterfaceLibrary() != null);
+ Builder() {}
- return autoBuild();
+ abstract AutoLibraryToLink autoBuild();
+
+ @Override
+ public final LibraryToLink build() {
+ LibraryToLink result = autoBuild();
+ Preconditions.checkNotNull(result.getLibraryIdentifier(), result);
+ Preconditions.checkState(
+ (result.getObjectFiles() == null
+ && result.getLtoCompilationContext() == null
+ && result.getSharedNonLtoBackends() == null)
+ || result.getStaticLibrary() != null,
+ result);
+ Preconditions.checkState(
+ (result.getPicObjectFiles() == null
+ && result.getPicLtoCompilationContext() == null
+ && result.getPicSharedNonLtoBackends() == null)
+ || result.getPicStaticLibrary() != null,
+ result);
+ Preconditions.checkState(
+ result.getResolvedSymlinkDynamicLibrary() == null || result.getDynamicLibrary() != null,
+ result);
+ Preconditions.checkState(
+ result.getResolvedSymlinkInterfaceLibrary() == null
+ || result.getResolvedSymlinkInterfaceLibrary() != null,
+ result);
+ Preconditions.checkState(
+ result.getStaticLibrary() != null
+ || result.getPicStaticLibrary() != null
+ || result.getDynamicLibrary() != null
+ || result.getInterfaceLibrary() != null,
+ result);
+
+ // Static-only instances must always return StaticOnlyLibraryToLink to preserve equality.
+ if (result.getStaticLibrary() != null
+ && !result.getAlwayslink()
+ && !result.getMustKeepDebug()
+ && !result.getDisableWholeArchive()
+ && result.getPicStaticLibrary() == null
+ && result.getDynamicLibrary() == null
+ && result.getInterfaceLibrary() == null
+ && result.getSharedNonLtoBackends() == null
+ && result.getPicObjectFiles() == null
+ && result.getPicLtoCompilationContext() == null) {
+ Artifact staticLibrary = result.getStaticLibrary();
+ String libraryIdentifier = result.getLibraryIdentifier();
+
+ // Try to reuse an existing instance if possible.
+ StaticOnlyLibraryToLink existing =
+ StaticOnlyLibraryToLink.cache.getIfPresent(staticLibrary);
+ if (existing != null && existing.getLibraryIdentifier().equals(libraryIdentifier)) {
+ return existing;
+ }
+
+ return new AutoValue_LibraryToLink_StaticOnlyLibraryToLink(
+ result.getLibraryIdentifier(), result.getStaticLibrary());
+ }
+
+ return result;
+ }
+ }
+ }
+
+ /**
+ * Specialized implementation for the case when only {@link #getStaticLibrary} is needed, to save
+ * memory compared to {@link AutoLibraryToLink}.
+ */
+ @AutoValue
+ abstract static class StaticOnlyLibraryToLink extends LibraryToLink {
+
+ // Essentially an interner, but keyed on Artifact to defer creating the string identifier.
+ private static final LoadingCache<Artifact, StaticOnlyLibraryToLink> cache =
+ CacheBuilder.newBuilder()
+ .concurrencyLevel(BlazeInterners.concurrencyLevel())
+ .weakValues()
+ .build(
+ CacheLoader.from(
+ artifact ->
+ new AutoValue_LibraryToLink_StaticOnlyLibraryToLink(
+ CcLinkingOutputs.libraryIdentifierOf(artifact), artifact)));
+
+ @Override // Remove @Nullable.
+ public abstract Artifact getStaticLibrary();
+
+ @Nullable
+ @Override
+ public ImmutableList<Artifact> getObjectFiles() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public ImmutableMap<Artifact, LtoBackendArtifacts> getSharedNonLtoBackends() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public LtoCompilationContext getLtoCompilationContext() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public ImmutableList<Artifact> getPicObjectFiles() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public ImmutableMap<Artifact, LtoBackendArtifacts> getPicSharedNonLtoBackends() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public LtoCompilationContext getPicLtoCompilationContext() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public Artifact getPicStaticLibrary() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public Artifact getDynamicLibrary() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public Artifact getResolvedSymlinkDynamicLibrary() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public Artifact getInterfaceLibrary() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public Artifact getResolvedSymlinkInterfaceLibrary() {
+ return null;
+ }
+
+ @Override
+ public boolean getAlwayslink() {
+ return false;
+ }
+
+ @Override
+ public AutoLibraryToLink.Builder toBuilder() {
+ return builder()
+ .setStaticLibrary(getStaticLibrary())
+ .setLibraryIdentifier(getLibraryIdentifier());
+ }
+
+ @Override
+ boolean getMustKeepDebug() {
+ return false;
+ }
+
+ @Override
+ boolean getDisableWholeArchive() {
+ return false;
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD b/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
index b89c883..a7cfdbb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/BUILD
@@ -45,6 +45,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/analysis/starlark/annotations",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/collect/compacthashset",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/events",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
index 779bd16..cbe134d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java
@@ -15,24 +15,27 @@
package com.google.devtools.build.lib.rules.objc;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.SymbolGenerator;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.cpp.CcCompilationContext;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkingContext;
import com.google.devtools.build.lib.rules.cpp.CcLinkingContext.LinkOptions;
-import com.google.devtools.build.lib.rules.cpp.CcLinkingOutputs;
+import com.google.devtools.build.lib.rules.cpp.CcLinkingContext.LinkerInput;
import com.google.devtools.build.lib.rules.cpp.LibraryToLink;
+import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.TreeMap;
/**
@@ -43,7 +46,7 @@
/**
* Constructs an {@link ObjcCommon} instance based on the attributes of the given rule context.
*/
- private ObjcCommon common(RuleContext ruleContext) throws InterruptedException {
+ private static ObjcCommon common(RuleContext ruleContext) throws InterruptedException {
return new ObjcCommon.Builder(ObjcCommon.Purpose.COMPILE_AND_LINK, ruleContext)
.setCompilationAttributes(
CompilationAttributes.Builder.fromRuleContext(ruleContext).build())
@@ -111,59 +114,64 @@
.build();
}
- private CcLinkingContext buildCcLinkingContext(
+ private static CcLinkingContext buildCcLinkingContext(
Label label, ObjcProvider objcProvider, SymbolGenerator<?> symbolGenerator) {
- ImmutableSet.Builder<LibraryToLink> libraries = new ImmutableSet.Builder<>();
- for (Artifact library : objcProvider.get(ObjcProvider.LIBRARY).toList()) {
- libraries.add(
- LibraryToLink.builder()
- .setStaticLibrary(library)
- .setLibraryIdentifier(CcLinkingOutputs.libraryIdentifierOf(library))
- .build());
+ List<Artifact> libraries = objcProvider.get(ObjcProvider.LIBRARY).toList();
+ List<LibraryToLink> ccLibraries = objcProvider.get(ObjcProvider.CC_LIBRARY).toList();
+
+ Set<LibraryToLink> librariesToLink =
+ CompactHashSet.createWithExpectedSize(libraries.size() + ccLibraries.size());
+ for (Artifact library : libraries) {
+ librariesToLink.add(LibraryToLink.staticOnly(library));
}
- libraries.addAll(
- convertLibrariesToStaticLibraries(objcProvider.get(ObjcProvider.CC_LIBRARY).toList()));
+ for (LibraryToLink library : ccLibraries) {
+ librariesToLink.add(convertToStaticLibrary(library));
+ }
- CcLinkingContext.Builder ccLinkingContext =
- CcLinkingContext.builder()
- .setOwner(label)
- .addLibraries(libraries.build().asList())
- .addLinkstamps(objcProvider.get(ObjcProvider.LINKSTAMP).toList());
-
- ImmutableList.Builder<LinkOptions> userLinkFlags = ImmutableList.builder();
- for (SdkFramework sdkFramework : objcProvider.get(ObjcProvider.SDK_FRAMEWORK).toList()) {
+ List<SdkFramework> sdkFrameworks = objcProvider.get(ObjcProvider.SDK_FRAMEWORK).toList();
+ ImmutableList.Builder<LinkOptions> userLinkFlags =
+ ImmutableList.builderWithExpectedSize(sdkFrameworks.size());
+ for (SdkFramework sdkFramework : sdkFrameworks) {
userLinkFlags.add(
LinkOptions.of(ImmutableList.of("-framework", sdkFramework.getName()), symbolGenerator));
}
- ccLinkingContext.addUserLinkFlags(userLinkFlags.build());
- return ccLinkingContext.build();
+ LinkerInput linkerInput =
+ new LinkerInput(
+ label,
+ ImmutableList.copyOf(librariesToLink),
+ userLinkFlags.build(),
+ /*nonCodeInputs=*/ ImmutableList.of(),
+ objcProvider.get(ObjcProvider.LINKSTAMP).toList());
+
+ return new CcLinkingContext(
+ NestedSetBuilder.create(Order.LINK_ORDER, linkerInput), /*extraLinkTimeLibraries=*/ null);
}
/**
- * This method removes dynamic libraries from LibraryToLink objects coming from C++ dependencies.
- * The reason for this is that objective-C rules do not support linking the dynamic version of the
+ * Removes dynamic libraries from {@link LibraryToLink} objects coming from C++ dependencies. The
+ * reason for this is that objective-C rules do not support linking the dynamic version of the
* libraries.
+ *
+ * <p>Returns the same object if nothing would be changed.
*/
- private ImmutableList<LibraryToLink> convertLibrariesToStaticLibraries(
- Iterable<LibraryToLink> librariesToLink) {
- ImmutableList.Builder<LibraryToLink> libraries = ImmutableList.builder();
- for (LibraryToLink libraryToLink : librariesToLink) {
- LibraryToLink.Builder staticLibraryToLink = libraryToLink.toBuilder();
- if (libraryToLink.getPicStaticLibrary() != null || libraryToLink.getStaticLibrary() != null) {
- staticLibraryToLink.setDynamicLibrary(null);
- staticLibraryToLink.setResolvedSymlinkDynamicLibrary(null);
- staticLibraryToLink.setInterfaceLibrary(null);
- staticLibraryToLink.setResolvedSymlinkInterfaceLibrary(null);
- }
- libraries.add(staticLibraryToLink.build());
+ private static LibraryToLink convertToStaticLibrary(LibraryToLink library) {
+ if ((library.getPicStaticLibrary() == null && library.getStaticLibrary() == null)
+ || (library.getDynamicLibrary() == null && library.getInterfaceLibrary() == null)) {
+ return library;
}
- return libraries.build();
+
+ return library.toBuilder()
+ .setDynamicLibrary(null)
+ .setResolvedSymlinkDynamicLibrary(null)
+ .setInterfaceLibrary(null)
+ .setResolvedSymlinkInterfaceLibrary(null)
+ .build();
}
/** Throws errors or warnings for bad attribute state. */
- private static void validateAttributes(RuleContext ruleContext) throws RuleErrorException {
+ private static void validateAttributes(RuleContext ruleContext) {
// TODO(b/129469095): objc_library cannot handle target names with slashes. Rather than
// crashing bazel, we emit a useful error message.
if (ruleContext.getTarget().getName().indexOf('/') != -1) {