Allow ObjectCodecRegistry to handle MemoizingCodecs. Initially this is just Skylark memoization, but we may extend in future to handle more than just Skylark this way. In fact, we probably want most of our ObjectCodecs to be MemoizingCodecs that can efficiently fall back to ObjectCodec if not using memoization (maybe coming in a follow-up).

At a high level, this CL merges the functionality of MemoizingCodecMap into ObjectCodecRegistry, adds auto-registration of MemoizingCodec, and uses that to get rid of a lot of codecs that were just delegating. The big one to get rid of there is SkylarkValueCodec: all of its delegation duties are implicitly now in ObjectCodecRegistry and friends.

One danger with this CL is that many of the features of Skylark serialization are only being tested in unit tests, which had to be reworked as part of this change. I don't think we've lost any coverage, but I could be wrong. SkylarkValueCodec had a bunch of methods that were effectively test-only, which made it easier to remove.

The plan is to provide a Memoizer.Serializer inside the SerializationContext. At the top level, it will be a DUMMY_SERIALIZER that does no memoization, but a MemoizingCodec can do
context = context.ensureMemoizing()
which will recreate the context with a true memoizing serializer. Then all references to the Serializer in codec code can be cleaned up, and the MemoizingCodec and ObjectCodec signatures will be the same. At that point, we can make all ObjectCodecs compatible with memoization by default (with strategy MEMOIZE_AFTER), and add a "memoize" boolean to @AutoCodec. That should allow us to have full interoperability between all codecs.

This CL also makes CodecScanner deterministic in the order of classes that it processes (there was a lurking bug here where constants must be deterministically ordered but that wasn't enforced at all).

PiperOrigin-RevId: 188559983

1 files changed, 6 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
index 878f1eba55..bf55bb43ce 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Interner;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -26,10 +27,11 @@ import com.google.devtools.build.skyframe.SkyValue;
 import java.util.Objects;
 
 /**
- * A value that represents a Skylark import lookup result. The lookup value corresponds to
- * exactly one Skylark file, identified by an absolute {@link Label} {@link SkyKey} argument. The
- * Label should not reference the special {@code external} package.
+ * A value that represents a Skylark import lookup result. The lookup value corresponds to exactly
+ * one Skylark file, identified by an absolute {@link Label} {@link SkyKey} argument. The Label
+ * should not reference the special {@code external} package.
  */
+@AutoCodec
 public class SkylarkImportLookupValue implements SkyValue {
 
   private final Extension environmentExtension;
@@ -40,6 +42,7 @@ public class SkylarkImportLookupValue implements SkyValue {
    */
   private final SkylarkFileDependency dependency;
 
+  @VisibleForTesting
   public SkylarkImportLookupValue(
       Extension environmentExtension, SkylarkFileDependency dependency) {
     this.environmentExtension = Preconditions.checkNotNull(environmentExtension);