Register `PackageSpecification.PackageGroupContents` for Skycache value-sharing
During Skycache frontier serialization, `PackageSpecification.PackageGroupContents` (and its generated `@AutoValue` subclass `AutoValue_PackageSpecification_PackageGroupContents`) was previously serialized inline via `DynamicCodec` whenever referenced by package visibility specifications or `NestedSet<PackageGroupContents>` nodes. Across a single build graph, 30,798 `PackageGroupContents` instances were serialized inline (despite only 2,861 unique instances), accounting for 3.14 MB (205,770 cumulative objects) of wire data.
This change:
- Annotates `PackageSpecification.PackageGroupContents` with `@AutoCodec` and adds a `@VisibleForSerialization @AutoCodec.Instantiator` factory method (`createForSerialization`) returning `AutoValue_PackageSpecification_PackageGroupContents`.
- Registers `PackageGroupContents.class` in `SerializationRegistrySetupHelpers.AUTOCODEC_CLASSES_FOR_VALUE_SHARING` so `PackageGroupContents` instances are deduplicated across Skyframe nodes via `ValueSharingAdapter`.
- Adds `PackageGroupTest.testPackageGroupContentsCodec` to verify round-trip serialization and reference-identity deduplication (`isSameInstanceAs`) with `SerializationRegistrySetupHelpers.analysisCachingCodecs()`.
Before:
```java
@AutoValue
public abstract static class PackageGroupContents
implements NestedSetsShouldBeInternedByEquality {
// Serialized inline via DynamicCodec without value-sharing across Skyframe nodes.
}
```
After:
```java
@AutoCodec
@AutoValue
public abstract static class PackageGroupContents
implements NestedSetsShouldBeInternedByEquality {
@VisibleForSerialization
@AutoCodec.Instantiator
static PackageGroupContents createForSerialization(...) {
return new AutoValue_PackageSpecification_PackageGroupContents(...);
}
}
```
PiperOrigin-RevId: 1008249881
Change-Id: I07a674af764898a5d48ce1ab98dc9fafca7ea709
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BUILD b/src/main/java/com/google/devtools/build/lib/packages/BUILD
index 92bf4cc..2780030 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/packages/BUILD
@@ -284,6 +284,7 @@
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset:nested_sets_should_be_interned_by_equality",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/net/starlark/java/eval",
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
index ceecbaf..bc5b64e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageSpecification.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSetsShouldBeInternedByEquality;
import com.google.devtools.build.lib.skyframe.serialization.VisibleForSerialization;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.vfs.PathFragment;
import javax.annotation.Nullable;
@@ -461,6 +462,7 @@
* <p>For modeling a {@code package_group}'s transitive contents (i.e., via the {@code includes}
* attribute), see {@link PackageSpecificationProvider}.
*/
+ @AutoCodec
@AutoValue
public abstract static class PackageGroupContents
implements NestedSetsShouldBeInternedByEquality {
@@ -547,7 +549,7 @@
throw new IllegalStateException(spec.toString());
}
- return new AutoValue_PackageSpecification_PackageGroupContents(
+ return createForSerialization(
singlePackagePositivesBuilder.build(),
allPackagesBeneathPositivesBuilder.build(),
hasPositiveAllPackages,
@@ -557,6 +559,26 @@
hasNegativeAllPackages);
}
+ @VisibleForSerialization
+ @AutoCodec.Instantiator
+ static PackageGroupContents createForSerialization(
+ ImmutableSet<PackageIdentifier> singlePackagePositives,
+ ImmutableList<PackageIdentifier> allPackagesBeneathPositives,
+ boolean hasPositiveAllPackages,
+ ImmutableSet<PackageIdentifier> singlePackageNegatives,
+ ImmutableList<PackageIdentifier> allPackagesBeneathNegatives,
+ boolean hasPrivate,
+ boolean hasNegativeAllPackages) {
+ return new AutoValue_PackageSpecification_PackageGroupContents(
+ singlePackagePositives,
+ allPackagesBeneathPositives,
+ hasPositiveAllPackages,
+ singlePackageNegatives,
+ allPackagesBeneathNegatives,
+ hasPrivate,
+ hasNegativeAllPackages);
+ }
+
/**
* Returns true if the given package matches at least one of this {@code PackageGroupContents}'
* positive specifications and none of its negative specifications.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD
index d2fe3bf..ae5960f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/BUILD
@@ -47,6 +47,7 @@
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/packages:package_specification",
"//src/main/java/com/google/devtools/build/lib/packages:provider",
"//src/main/java/com/google/devtools/build/lib/rules:alias",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationRegistrySetupHelpers.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationRegistrySetupHelpers.java
index c6e0dfa..09a0f58 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationRegistrySetupHelpers.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationRegistrySetupHelpers.java
@@ -41,6 +41,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.nestedset.DeferredNestedSetCodec;
import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment;
+import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.packages.StructProvider;
import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
@@ -170,7 +171,8 @@
AliasConfiguredTarget.class,
BuildConfigurationValue.class,
InlineFileArtifactValue.class,
- AspectKeyCreator.AspectKey.class);
+ AspectKeyCreator.AspectKey.class,
+ PackageGroupContents.class);
private static final ImmutableList<ObjectCodec<?>> INSTANCE;
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD
index 943c860..c0e694f 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD
@@ -79,6 +79,8 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
"//src/main/java/com/google/devtools/build/lib/skyframe:tests_for_target_pattern_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:serialization_registry_setup_helpers",
+ "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils:round-tripping",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
index ab64ae1..8fbfecd 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageGroupTest.java
@@ -22,6 +22,9 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
+import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec;
+import com.google.devtools.build.lib.skyframe.serialization.SerializationRegistrySetupHelpers;
+import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -542,6 +545,42 @@
assertThat(grp).hasSamePropertiesAs(grp.reduceForSerialization());
}
+ @Test
+ public void testPackageGroupContentsCodec() throws Exception {
+ RepositoryName mainRepository = RepositoryName.MAIN;
+ RepositoryName otherRepository = RepositoryName.create("other");
+ PackageGroupContents packageGroupContents =
+ PackageGroupContents.create(
+ ImmutableList.of(
+ pkgSpec(mainRepository, "//a"),
+ pkgSpec(mainRepository, "//b/c/..."),
+ pkgSpec(mainRepository, "-//d/e"),
+ pkgSpec(mainRepository, "-//f/..."),
+ pkgSpec(otherRepository, "//z/..."),
+ pkgSpec(mainRepository, "private")));
+
+ SerializationTester serializationTester =
+ new SerializationTester(
+ packageGroupContents,
+ PackageGroupContents.create(ImmutableList.of(pkgSpec(mainRepository, "public"))),
+ PackageGroupContents.create(ImmutableList.of(pkgSpec(mainRepository, "-//..."))),
+ ImmutableList.of(packageGroupContents, packageGroupContents))
+ .makeMemoizingAndAllowFutureBlocking(true)
+ .setVerificationFunction(
+ (originalSubject, deserializedSubject) -> {
+ assertThat(deserializedSubject).isEqualTo(originalSubject);
+ if (deserializedSubject instanceof ImmutableList<?> deserializedContentsList) {
+ assertThat(deserializedContentsList.get(0))
+ .isSameInstanceAs(deserializedContentsList.get(1));
+ }
+ });
+ for (ObjectCodec<?> analysisCachingCodec :
+ SerializationRegistrySetupHelpers.analysisCachingCodecs()) {
+ serializationTester.addCodec(analysisCachingCodec);
+ }
+ serializationTester.runTests();
+ }
+
/** Convenience method for obtaining a PackageSpecification. */
private PackageSpecification pkgSpec(RepositoryName repository, String spec) throws Exception {
return PackageSpecification.fromString(
