Skip to content

Commit

Permalink
Optimize shallow heap of LibraryToLinkValue.
Browse files Browse the repository at this point in the history 
Currently the shallow heap size is `32` bytes (`12` bytes for the `Object` header, `1` boolean field, and `3` reference fields => `12 + 1 + 3*4 = 25`, which rounds up to `32`, the next multiple of `8`). But the 6 different factory methods don't actually need all those fields. Instead, we can save shallow heap by using a class hierarchy to encode information.

For a large Bazel invocation with ~1M `LibraryToLinkValue` instances this CL saves ~16MB in the common situation where calls to `#forObjectFileGroup` are uncommon.

While I was here I wrote unit tests for `LibraryToLinkValue` so that the next person who looks at the `#getFieldValue` business logic can more easily make changes.

PiperOrigin-RevId: 432471316
  • Loading branch information
@haxorz @copybara-github
haxorz authored and copybara-github committed Mar 4, 2022
1 parent c1db56c commit ae41448
Show file tree
Hide file tree
Showing 3 changed files with 646 additions and 89 deletions.
298 changes: 209 additions & 89 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,123 +656,62 @@ public StructureValue build() {
* significantly reduces memory overhead.
*/
@Immutable
public static class LibraryToLinkValue extends VariableValueAdapter {
public abstract static class LibraryToLinkValue extends VariableValueAdapter {
public static final String OBJECT_FILES_FIELD_NAME = "object_files";
public static final String NAME_FIELD_NAME = "name";
public static final String TYPE_FIELD_NAME = "type";
public static final String IS_WHOLE_ARCHIVE_FIELD_NAME = "is_whole_archive";

private static final String LIBRARY_TO_LINK_VARIABLE_TYPE_NAME = "structure (LibraryToLink)";

@VisibleForSerialization
enum Type {
OBJECT_FILE("object_file"),
OBJECT_FILE_GROUP("object_file_group"),
INTERFACE_LIBRARY("interface_library"),
STATIC_LIBRARY("static_library"),
DYNAMIC_LIBRARY("dynamic_library"),
VERSIONED_DYNAMIC_LIBRARY("versioned_dynamic_library");

private final String name;

Type(String name) {
this.name = name;
}
}

private final String name;
private final ImmutableList<Artifact> objectFiles;
private final boolean isWholeArchive;
private final Type type;

public static LibraryToLinkValue forDynamicLibrary(String name) {
return new LibraryToLinkValue(
Preconditions.checkNotNull(name),
/* objectFiles= */ null,
/* isWholeArchive= */ false,
Type.DYNAMIC_LIBRARY);
return new ForDynamicLibrary(name);
}

public static LibraryToLinkValue forVersionedDynamicLibrary(String name) {
return new LibraryToLinkValue(
Preconditions.checkNotNull(name),
/* objectFiles= */ null,
/* isWholeArchive= */ false,
Type.VERSIONED_DYNAMIC_LIBRARY);
return new ForVersionedDynamicLibrary(name);
}

public static LibraryToLinkValue forInterfaceLibrary(String name) {
return new LibraryToLinkValue(
Preconditions.checkNotNull(name),
/* objectFiles= */ null,
/* isWholeArchive= */ false,
Type.INTERFACE_LIBRARY);
return new ForInterfaceLibrary(name);
}

public static LibraryToLinkValue forStaticLibrary(String name, boolean isWholeArchive) {
return new LibraryToLinkValue(
Preconditions.checkNotNull(name),
/* objectFiles= */ null,
isWholeArchive,
Type.STATIC_LIBRARY);
return isWholeArchive ? new ForStaticLibraryWholeArchive(name) : new ForStaticLibrary(name);
}

public static LibraryToLinkValue forObjectFile(String name, boolean isWholeArchive) {
return new LibraryToLinkValue(
Preconditions.checkNotNull(name),
/* objectFiles= */ null,
isWholeArchive,
Type.OBJECT_FILE);
return isWholeArchive ? new ForObjectFileWholeArchive(name) : new ForObjectFile(name);
}

public static LibraryToLinkValue forObjectFileGroup(
ImmutableList<Artifact> objects, boolean isWholeArchive) {
Preconditions.checkNotNull(objects);
Preconditions.checkArgument(!objects.isEmpty());
return new LibraryToLinkValue(
/* name= */ null, objects, isWholeArchive, Type.OBJECT_FILE_GROUP);
}

@VisibleForSerialization
LibraryToLinkValue(
String name, ImmutableList<Artifact> objectFiles, boolean isWholeArchive, Type type) {
this.name = name;
this.objectFiles = objectFiles;
this.isWholeArchive = isWholeArchive;
this.type = type;
return isWholeArchive
? new ForObjectFileGroupWholeArchive(objects)
: new ForObjectFileGroup(objects);
}

@Override
public VariableValue getFieldValue(
String variableName,
String field,
@Nullable ArtifactExpander expander,
boolean throwOnMissingVariable) {
Preconditions.checkNotNull(field);
if (NAME_FIELD_NAME.equals(field) && !type.equals(Type.OBJECT_FILE_GROUP)) {
return new StringValue(name);
} else if (OBJECT_FILES_FIELD_NAME.equals(field) && type.equals(Type.OBJECT_FILE_GROUP)) {
ImmutableList.Builder<String> expandedObjectFiles = ImmutableList.builder();
for (Artifact objectFile : objectFiles) {
if (objectFile.isTreeArtifact() && (expander != null)) {
List<Artifact> artifacts = new ArrayList<>();
expander.expand(objectFile, artifacts);
expandedObjectFiles.addAll(
Iterables.transform(artifacts, artifact -> artifact.getExecPathString()));
} else {
expandedObjectFiles.add(objectFile.getExecPathString());
}
}
return new StringSequence(expandedObjectFiles.build());
} else if (TYPE_FIELD_NAME.equals(field)) {
return new StringValue(type.name);
if (TYPE_FIELD_NAME.equals(field)) {
return new StringValue(getTypeName());
} else if (IS_WHOLE_ARCHIVE_FIELD_NAME.equals(field)) {
return new IntegerValue(isWholeArchive ? 1 : 0);
} else {
return null;
return new IntegerValue(getIsWholeArchive() ? 1 : 0);
}
return null;
}

protected boolean getIsWholeArchive() {
return false;
}

protected abstract String getTypeName();

@Override
public String getVariableTypeName() {
return LIBRARY_TO_LINK_VARIABLE_TYPE_NAME;
Expand All @@ -784,23 +723,204 @@ public boolean isTruthy() {
}

@Override
public boolean equals(Object other) {
if (!(other instanceof LibraryToLinkValue)) {
public boolean equals(Object obj) {
if (!(obj instanceof LibraryToLinkValue)) {
return false;
}
if (this == other) {
if (this == obj) {
return true;
}
LibraryToLinkValue that = (LibraryToLinkValue) other;
return Objects.equals(this.name, that.name)
&& Objects.equals(this.objectFiles, that.objectFiles)
&& this.isWholeArchive == that.isWholeArchive
&& Objects.equals(this.type, that.type);
LibraryToLinkValue other = (LibraryToLinkValue) obj;
return this.getTypeName().equals(other.getTypeName())
&& getIsWholeArchive() == other.getIsWholeArchive();
}

@Override
public int hashCode() {
return 31 * Objects.hash(name, objectFiles, type) + (isWholeArchive ? 1231 : 1237);
return Objects.hash(getTypeName(), getIsWholeArchive());
}

private abstract static class LibraryToLinkValueWithName extends LibraryToLinkValue {
private final String name;

LibraryToLinkValueWithName(String name) {
this.name = Preconditions.checkNotNull(name);
}

@Override
public VariableValue getFieldValue(
String variableName,
String field,
@Nullable ArtifactExpander expander,
boolean throwOnMissingVariable) {
if (NAME_FIELD_NAME.equals(field)) {
return new StringValue(name);
}
return super.getFieldValue(variableName, field, expander, throwOnMissingVariable);
}

@Override
public boolean equals(Object obj) {
if (!(obj instanceof LibraryToLinkValueWithName)) {
return false;
}
if (this == obj) {
return true;
}
LibraryToLinkValueWithName other = (LibraryToLinkValueWithName) obj;
return this.name.equals(other.name) && super.equals(other);
}

@Override
public int hashCode() {
return 31 * super.hashCode() + name.hashCode();
}
}

private static final class ForDynamicLibrary extends LibraryToLinkValueWithName {
private ForDynamicLibrary(String name) {
super(name);
}

@Override
protected String getTypeName() {
return "dynamic_library";
}
}

private static final class ForVersionedDynamicLibrary extends LibraryToLinkValueWithName {
private ForVersionedDynamicLibrary(String name) {
super(name);
}

@Override
protected String getTypeName() {
return "versioned_dynamic_library";
}
}

private static final class ForInterfaceLibrary extends LibraryToLinkValueWithName {
private ForInterfaceLibrary(String name) {
super(name);
}

@Override
protected String getTypeName() {
return "interface_library";
}
}

private static class ForStaticLibrary extends LibraryToLinkValueWithName {
private ForStaticLibrary(String name) {
super(name);
}

@Override
protected String getTypeName() {
return "static_library";
}
}

private static final class ForStaticLibraryWholeArchive extends ForStaticLibrary {
private ForStaticLibraryWholeArchive(String name) {
super(name);
}

@Override
protected boolean getIsWholeArchive() {
return true;
}
}

private static class ForObjectFile extends LibraryToLinkValueWithName {
private ForObjectFile(String name) {
super(name);
}

@Override
protected String getTypeName() {
return "object_file";
}
}

private static final class ForObjectFileWholeArchive extends ForObjectFile {
private ForObjectFileWholeArchive(String name) {
super(name);
}

@Override
protected boolean getIsWholeArchive() {
return true;
}
}

private static class ForObjectFileGroup extends LibraryToLinkValue {
private final ImmutableList<Artifact> objectFiles;

private ForObjectFileGroup(ImmutableList<Artifact> objectFiles) {
this.objectFiles = objectFiles;
}

@Override
public VariableValue getFieldValue(
String variableName,
String field,
@Nullable ArtifactExpander expander,
boolean throwOnMissingVariable) {
if (NAME_FIELD_NAME.equals(field)) {
return null;
}

if (OBJECT_FILES_FIELD_NAME.equals(field)) {
ImmutableList.Builder<String> expandedObjectFiles = ImmutableList.builder();
for (Artifact objectFile : objectFiles) {
if (objectFile.isTreeArtifact() && (expander != null)) {
List<Artifact> artifacts = new ArrayList<>();
expander.expand(objectFile, artifacts);
expandedObjectFiles.addAll(
Iterables.transform(artifacts, artifact -> artifact.getExecPathString()));
} else {
expandedObjectFiles.add(objectFile.getExecPathString());
}
}
return new StringSequence(expandedObjectFiles.build());
}

return super.getFieldValue(variableName, field, expander, throwOnMissingVariable);
}

@Override
protected String getTypeName() {
return "object_file_group";
}

@Override
public boolean equals(Object obj) {
if (!(obj instanceof ForObjectFileGroup)) {
return false;
}
if (this == obj) {
return true;
}
ForObjectFileGroup other = (ForObjectFileGroup) obj;
return this.objectFiles.equals(other.objectFiles) && super.equals(other);
}

@Override
public int hashCode() {
return 31 * super.hashCode() + objectFiles.hashCode();
}
}

private static final class ForObjectFileGroupWholeArchive extends ForObjectFileGroup {
private ForObjectFileGroupWholeArchive(ImmutableList<Artifact> objectFiles) {
super(objectFiles);
}

@Override
protected boolean getIsWholeArchive() {
return true;
}
}
}

Expand Down
17 changes: 17 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -740,3 +740,20 @@ java_test(
"//third_party:junit4",
],
)

java_test(
name = "LibraryToLinkValueTest",
srcs = ["LibraryToLinkValueTest.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/actions:artifact_owner",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
"//third_party:guava",
"//third_party:guava-testlib",
"//third_party:junit4",
"//third_party:truth",
],
)
Loading

0 comments on commit ae41448

Please sign in to comment.