Move createAnalysisCodecRegistrySupplier to CommonSerializationConsta…

…nts. ... and rename the class to be more general (SerializationRegistrySetupHelpers). PiperOrigin-RevId: 663317765 Change-Id: I28738401a034864621b8f07e46d5f9da12aad4d8
tsawada · Aug 15, 2024 · 52ac2dd · 52ac2dd
1 parent cda1a28
commit 52ac2dd
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 51 deletions.
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
@@ -20,19 +20,24 @@ java_library(
 )
 
 java_library(
-    name = "common_serialization_constants",
-    srcs = ["CommonSerializationConstants.java"],
+    name = "serialization_registry_setup_helpers",
+    srcs = ["SerializationRegistrySetupHelpers.java"],
     deps = [
         ":serialization",
+        "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
+        "//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/skyframe:configured_target_key",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
+        "//third_party:error_prone_annotations",
         "//third_party:guava",
     ],
 )
@@ -41,15 +46,9 @@ java_library(
     name = "serialization_module",
     srcs = ["SerializationModule.java"],
     deps = [
-        ":common_serialization_constants",
-        ":serialization",
+        ":serialization_registry_setup_helpers",
         "//src/main/java/com/google/devtools/build/lib:runtime",
-        "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
-        "//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/skyframe:configured_target_key",
-        "//third_party:guava",
     ],
 )
 
@@ -60,7 +59,7 @@ java_library(
         exclude = [
             "AbstractExportedStarlarkSymbolCodec.java",
             "CodecScanningConstants.java",
-            "CommonSerializationConstants.java",
+            "SerializationRegistrySetupHelpers.java",
             "SerializationConstants.java",
             "SerializationModule.java",
             "VisibleForSerialization.java",

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationModule.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationModule.java
@@ -13,18 +13,10 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe.serialization;
 
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.actions.ArtifactCodecs;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-import com.google.devtools.build.lib.collect.nestedset.DeferredNestedSetCodec;
 import com.google.devtools.build.lib.runtime.BlazeModule;
 import com.google.devtools.build.lib.runtime.BlazeRuntime;
 import com.google.devtools.build.lib.runtime.WorkspaceBuilder;
-import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
-import java.util.function.Supplier;
 
 /** A {@link BlazeModule} to store Skyframe serialization lifecycle hooks. */
 public class SerializationModule extends BlazeModule {
@@ -42,40 +34,12 @@ public void workspaceInit(
     // This is injected as a callback instead of evaluated eagerly to avoid forcing the somewhat
     // expensive AutoRegistry.get call on clients that don't require it.
     runtime.initAnalysisCodecRegistry(
-        createAnalysisCodecRegistrySupplier(
+        SerializationRegistrySetupHelpers.createAnalysisCodecRegistrySupplier(
             runtime,
-            CommonSerializationConstants.makeReferenceConstants(
+            SerializationRegistrySetupHelpers.makeReferenceConstants(
                 directories,
                 runtime.getRuleClassProvider(),
                 directories.getWorkspace().getBaseName())));
   }
 
-  /**
-   * Initializes an {@link ObjectCodecRegistry} for analysis serialization.
-   *
-   * <p>This gets injected into {@link BlazeRuntime} and made available to clients via {@link
-   * BlazeRuntime#getAnalysisCodecRegistry}.
-   *
-   * <p>TODO: move this to CommonSerializationConstants instead.
-   */
-  protected static Supplier<ObjectCodecRegistry> createAnalysisCodecRegistrySupplier(
-      BlazeRuntime runtime, ImmutableList<Object> additionalReferenceConstants) {
-    return () -> {
-      ObjectCodecRegistry.Builder builder =
-          AutoRegistry.get()
-              .getBuilder()
-              .addReferenceConstants(additionalReferenceConstants)
-              .computeChecksum(false)
-              .add(ArrayCodec.forComponentType(Artifact.class))
-              .add(new DeferredNestedSetCodec())
-              .add(Label.valueSharingCodec())
-              .add(PackageIdentifier.valueSharingCodec())
-              .add(ConfiguredTargetKey.valueSharingCodec());
-      builder =
-          CommonSerializationConstants.addStarlarkFunctionality(
-              builder, runtime.getRuleClassProvider());
-      ArtifactCodecs.VALUE_SHARING_CODECS.forEach(builder::add);
-      return builder.build();
-    };
-  }
 }
diff --git a/...ization/CommonSerializationConstants.java → ...on/SerializationRegistrySetupHelpers.java b/...ization/CommonSerializationConstants.java → ...on/SerializationRegistrySetupHelpers.java
@@ -15,22 +15,36 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSortedMap;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactCodecs;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.config.OutputDirectories.OutputDirectory;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
+import com.google.devtools.build.lib.cmdline.Label;
+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.StructProvider;
+import com.google.devtools.build.lib.runtime.BlazeRuntime;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.vfs.Root;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.util.function.Supplier;
 import net.starlark.java.eval.Starlark;
 import net.starlark.java.syntax.Location;
 
-/** Objects that should be serialized as constants in Skyframe serialization. */
-public final class CommonSerializationConstants {
+/**
+ * Helpers for setting up the serialization registry (e.g. explicit codecs and constants).
+ *
+ * <p>The vast majority of codecs are automatically registered (see {@link AutoRegistry} and {@link
+ * CodecScanner}). This class provides methods to register additional codecs and constants,
+ * depending on the usage context.
+ */
+public final class SerializationRegistrySetupHelpers {
 
-  private CommonSerializationConstants() {}
+  private SerializationRegistrySetupHelpers() {}
 
   private static final ImmutableList<String> OUTPUT_PATHS =
       ImmutableList.of("k8-opt", "k8-fastbuild", "k8-debug");
@@ -104,4 +118,29 @@ public static ObjectCodecRegistry.Builder addStarlarkFunctionality(
 
     return builder;
   }
+
+  /**
+   * Initializes an {@link ObjectCodecRegistry} for analysis serialization.
+   *
+   * <p>This gets injected into {@link BlazeRuntime} and made available to clients via {@link
+   * BlazeRuntime#getAnalysisCodecRegistry}.
+   */
+  public static Supplier<ObjectCodecRegistry> createAnalysisCodecRegistrySupplier(
+      BlazeRuntime runtime, ImmutableList<Object> additionalReferenceConstants) {
+    return () -> {
+      ObjectCodecRegistry.Builder builder =
+          AutoRegistry.get()
+              .getBuilder()
+              .addReferenceConstants(additionalReferenceConstants)
+              .computeChecksum(false)
+              .add(ArrayCodec.forComponentType(Artifact.class))
+              .add(new DeferredNestedSetCodec())
+              .add(Label.valueSharingCodec())
+              .add(PackageIdentifier.valueSharingCodec())
+              .add(ConfiguredTargetKey.valueSharingCodec());
+      builder = addStarlarkFunctionality(builder, runtime.getRuleClassProvider());
+      ArtifactCodecs.VALUE_SHARING_CODECS.forEach(builder::add);
+      return builder.build();
+    };
+  }
 }