diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
13 files changed, 151 insertions, 206 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java index 2c8811559a..d58f350594 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java @@ -44,7 +44,7 @@ import java.util.ArrayList; import javax.annotation.Nullable; /** An aspect in the context of the Skyframe graph. */ -@AutoCodec(memoization = AutoCodec.Memoization.START_MEMOIZING) +@AutoCodec public final class AspectValue extends BasicActionLookupValue { /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java index f15f7d09a6..1d9d1a6cb8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/NonRuleConfiguredTargetValue.java @@ -35,7 +35,7 @@ import javax.annotation.Nullable; /** A non-rule configured target in the context of a Skyframe graph. */ @Immutable @ThreadSafe -@AutoCodec(memoization = AutoCodec.Memoization.START_MEMOIZING) +@AutoCodec @VisibleForTesting public final class NonRuleConfiguredTargetValue extends BasicActionLookupValue implements ConfiguredTargetValue { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageValue.java index 565e4f1290..ba1962335b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageValue.java @@ -30,7 +30,7 @@ import java.util.ArrayList; import java.util.List; /** A Skyframe value representing a package. */ -@AutoCodec(memoization = AutoCodec.Memoization.START_MEMOIZING) +@AutoCodec @Immutable @ThreadSafe public class PackageValue implements NotComparableSkyValue { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java index b48f247feb..359e638256 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java @@ -26,14 +26,13 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Memoization; import java.util.ArrayList; import javax.annotation.Nullable; /** A configured target in the context of a Skyframe graph. */ @Immutable @ThreadSafe -@AutoCodec(memoization = Memoization.START_MEMOIZING) +@AutoCodec @VisibleForTesting public final class RuleConfiguredTargetValue extends ActionLookupValue implements ConfiguredTargetValue { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java index 86aea0408f..e9bacf38fa 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java @@ -19,7 +19,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.skyframe.serialization.Memoizer.Deserializer; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecRegistry.CodecDescriptor; -import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs.MemoizationPermission; import com.google.protobuf.CodedInputStream; import java.io.IOException; import javax.annotation.CheckReturnValue; @@ -33,24 +32,21 @@ import javax.annotation.CheckReturnValue; public class DeserializationContext { private final ObjectCodecRegistry registry; private final ImmutableMap<Class<?>, Object> dependencies; - private final MemoizationPermission memoizationPermission; private final Memoizer.Deserializer deserializer; private DeserializationContext( ObjectCodecRegistry registry, ImmutableMap<Class<?>, Object> dependencies, - MemoizationPermission memoizationPermission, Deserializer deserializer) { this.registry = registry; this.dependencies = dependencies; - this.memoizationPermission = memoizationPermission; this.deserializer = deserializer; } @VisibleForTesting public DeserializationContext( ObjectCodecRegistry registry, ImmutableMap<Class<?>, Object> dependencies) { - this(registry, dependencies, MemoizationPermission.ALLOWED, /*deserializer=*/ null); + this(registry, dependencies, /*deserializer=*/ null); } @VisibleForTesting @@ -58,42 +54,6 @@ public class DeserializationContext { this(AutoRegistry.get(), dependencies); } - DeserializationContext disableMemoization() { - Preconditions.checkState( - memoizationPermission == MemoizationPermission.ALLOWED, "memoization already disabled"); - Preconditions.checkState(deserializer == null, "deserializer already present"); - return new DeserializationContext( - registry, dependencies, MemoizationPermission.DISABLED, deserializer); - } - - /** - * Returns a {@link DeserializationContext} that will memoize values it encounters (using - * reference equality), the inverse of the memoization performed by a {@link SerializationContext} - * returned by {@link SerializationContext#getMemoizingContext}. The context returned here should - * be used instead of the original: memoization may only occur when using the returned context. - * - * <p>This method is idempotent: calling it on an already memoizing context will return the same - * context. - */ - @CheckReturnValue - public DeserializationContext getMemoizingContext() { - Preconditions.checkState( - memoizationPermission == MemoizationPermission.ALLOWED, "memoization disabled"); - if (deserializer != null) { - return this; - } - return new DeserializationContext( - this.registry, this.dependencies, memoizationPermission, new Deserializer()); - } - - /** - * Register an initial value for the currently deserializing value, for use by child objects that - * may have references to it. Only for use during memoizing deserialization. - */ - public <T> void registerInitialValue(T initialValue) { - deserializer.registerInitialValue(initialValue); - } - // TODO(shahan): consider making codedIn a member of this class. @SuppressWarnings({"TypeParameterUnusedInFormals", "unchecked"}) public <T> T deserialize(CodedInputStream codedIn) throws IOException, SerializationException { @@ -113,9 +73,42 @@ public class DeserializationContext { } } + /** + * Register an initial value for the currently deserializing value, for use by child objects that + * may have references to it. + * + * <p>This is a noop when memoization is disabled. + */ + public <T> void registerInitialValue(T initialValue) { + if (deserializer == null) { + return; + } + deserializer.registerInitialValue(initialValue); + } + @SuppressWarnings("unchecked") public <T> T getDependency(Class<T> type) { Preconditions.checkNotNull(type); return (T) dependencies.get(type); } + + /** + * Returns a {@link DeserializationContext} that will memoize values it encounters (using + * reference equality), the inverse of the memoization performed by a {@link SerializationContext} + * returned by {@link SerializationContext#getMemoizingContext}. The context returned here should + * be used instead of the original: memoization may only occur when using the returned context. + * + * <p>This method is idempotent: calling it on an already memoizing context will return the same + * context. + * + * <p><em>This is public for testing and {@link + * com.google.devtools.build.lib.packages.PackageSerializer} only.</em> + */ + @CheckReturnValue + public DeserializationContext getMemoizingContext() { + if (deserializer != null) { + return this; + } + return new DeserializationContext(this.registry, this.dependencies, new Deserializer()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java index 4128092f1e..4ec874b03e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/DynamicCodec.java @@ -38,18 +38,11 @@ public class DynamicCodec implements ObjectCodec<Object> { private final Class<?> type; private final Constructor<?> constructor; private final ImmutableSortedMap<Field, Long> offsets; - private final ObjectCodec.MemoizationStrategy strategy; public DynamicCodec(Class<?> type) throws ReflectiveOperationException { - this(type, ObjectCodec.MemoizationStrategy.MEMOIZE_BEFORE); - } - - public DynamicCodec(Class<?> type, ObjectCodec.MemoizationStrategy strategy) - throws ReflectiveOperationException { this.type = type; this.constructor = getConstructor(type); this.offsets = getOffsets(type); - this.strategy = strategy; } @Override @@ -59,7 +52,7 @@ public class DynamicCodec implements ObjectCodec<Object> { @Override public MemoizationStrategy getStrategy() { - return strategy; + return ObjectCodec.MemoizationStrategy.MEMOIZE_BEFORE; } @Override @@ -141,9 +134,7 @@ public class DynamicCodec implements ObjectCodec<Object> { } catch (ReflectiveOperationException e) { throw new SerializationException("Could not instantiate object of type: " + type, e); } - if (strategy.equals(ObjectCodec.MemoizationStrategy.MEMOIZE_BEFORE)) { - context.registerInitialValue(instance); - } + context.registerInitialValue(instance); for (Map.Entry<Field, Long> entry : offsets.entrySet()) { deserializeField(context, codedIn, instance, entry.getKey().getType(), entry.getValue()); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecs.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecs.java index 70e450519b..3674901826 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecs.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecs.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.skyframe.serialization; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; import com.google.protobuf.CodedInputStream; @@ -26,8 +25,6 @@ import java.io.IOException; * serving as a layer between the streaming-oriented {@link ObjectCodec} interface and users. */ public class ObjectCodecs { - // TODO(shahan): when per-invocation state is needed, for example, memoization, these may - // need to be constructed each time. private final SerializationContext serializationContext; private final DeserializationContext deserializationContext; @@ -42,66 +39,81 @@ public class ObjectCodecs { } public ByteString serialize(Object subject) throws SerializationException { - ByteString.Output resultOut = ByteString.newOutput(); - CodedOutputStream codedOut = CodedOutputStream.newInstance(resultOut); - try { - serializationContext.serialize(subject, codedOut); - codedOut.flush(); - return resultOut.toByteString(); - } catch (IOException e) { - throw new SerializationException("Failed to serialize " + subject, e); - } + return serializeToByteString(subject, this::serialize); } - public void serialize( - Object subject, CodedOutputStream codedOut, MemoizationPermission memoizationPermission) - throws SerializationException { - SerializationContext context = serializationContext; - if (memoizationPermission == MemoizationPermission.DISABLED) { - context = context.disableMemoization(); - } - try { - context.serialize(subject, codedOut); - } catch (IOException e) { - throw new SerializationException("Failed to serialize " + subject, e); - } + public void serialize(Object subject, CodedOutputStream codedOut) throws SerializationException { + serializeImpl(subject, codedOut, /*memoize=*/ false); } - /** - * Controls whether memoization can occur for serialization/deserialization. Should be allowed - * unless bit-equivalence is needed. - */ - public enum MemoizationPermission { - ALLOWED, - DISABLED + public ByteString serializeMemoized(Object subject) throws SerializationException { + return serializeToByteString(subject, this::serializeMemoized); + } + + public void serializeMemoized(Object subject, CodedOutputStream codedOut) + throws SerializationException { + serializeImpl(subject, codedOut, /*memoize=*/ true); } public Object deserialize(ByteString data) throws SerializationException { - return deserialize(data.newCodedInput(), MemoizationPermission.ALLOWED); + return deserialize(data.newCodedInput()); + } + + public Object deserialize(CodedInputStream codedIn) throws SerializationException { + return deserializeImpl(codedIn, /*memoize=*/ false); } - public Object deserialize(CodedInputStream codedIn, MemoizationPermission memoizationPermission) + public Object deserializeMemoized(ByteString data) throws SerializationException { + return deserializeMemoized(data.newCodedInput()); + } + + public Object deserializeMemoized(CodedInputStream codedIn) throws SerializationException { + return deserializeImpl(codedIn, /*memoize=*/ true); + } + + private void serializeImpl(Object subject, CodedOutputStream codedOut, boolean memoize) throws SerializationException { - // Allow access to buffer without copying (although this means buffer may be pinned in memory). - codedIn.enableAliasing(true); - DeserializationContext context = deserializationContext; - if (memoizationPermission == MemoizationPermission.DISABLED) { - context = context.disableMemoization(); + try { + if (memoize) { + serializationContext.getMemoizingContext().serialize(subject, codedOut); + } else { + serializationContext.serialize(subject, codedOut); + } + } catch (IOException e) { + throw new SerializationException("Failed to serialize " + subject, e); } + } + + private Object deserializeImpl(CodedInputStream codedIn, boolean memoize) + throws SerializationException { + // Allows access to buffer without copying (although this means buffer may be pinned in memory). + codedIn.enableAliasing(true); try { - return context.deserialize(codedIn); + if (memoize) { + return deserializationContext.getMemoizingContext().deserialize(codedIn); + } else { + return deserializationContext.deserialize(codedIn); + } } catch (IOException e) { throw new SerializationException("Failed to deserialize data", e); } } - @VisibleForTesting - public SerializationContext getSerializationContextForTesting() { - return serializationContext; + @FunctionalInterface + private static interface SerializeCall { + void serialize(Object subject, CodedOutputStream codedOut) throws SerializationException; } - @VisibleForTesting - public DeserializationContext getDeserializationContextForTesting() { - return deserializationContext; + private static ByteString serializeToByteString(Object subject, SerializeCall wrapped) + throws SerializationException { + ByteString.Output resultOut = ByteString.newOutput(); + CodedOutputStream codedOut = CodedOutputStream.newInstance(resultOut); + wrapped.serialize(subject, codedOut); + try { + codedOut.flush(); + return resultOut.toByteString(); + } catch (IOException e) { + throw new SerializationException("Failed to serialize " + subject, e); + } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java index 6e0d6811d9..e06d6dba45 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java @@ -18,7 +18,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.skyframe.serialization.Memoizer.Serializer; -import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs.MemoizationPermission; import com.google.devtools.build.lib.skyframe.serialization.SerializationException.NoCodecException; import com.google.protobuf.CodedOutputStream; import java.io.IOException; @@ -34,26 +33,21 @@ import javax.annotation.Nullable; public class SerializationContext { private final ObjectCodecRegistry registry; private final ImmutableMap<Class<?>, Object> dependencies; - private final MemoizationPermission memoizationPermission; @Nullable private final Memoizer.Serializer serializer; private SerializationContext( ObjectCodecRegistry registry, ImmutableMap<Class<?>, Object> dependencies, - MemoizationPermission memoizationPermission, @Nullable Serializer serializer) { this.registry = registry; this.dependencies = dependencies; this.serializer = serializer; - this.memoizationPermission = memoizationPermission; - Preconditions.checkState( - serializer == null || memoizationPermission == MemoizationPermission.ALLOWED); } @VisibleForTesting public SerializationContext( ObjectCodecRegistry registry, ImmutableMap<Class<?>, Object> dependencies) { - this(registry, dependencies, MemoizationPermission.ALLOWED, /*serializer=*/ null); + this(registry, dependencies, /*serializer=*/ null); } @VisibleForTesting @@ -61,12 +55,26 @@ public class SerializationContext { this(AutoRegistry.get(), dependencies); } - SerializationContext disableMemoization() { - Preconditions.checkState( - memoizationPermission == MemoizationPermission.ALLOWED, "memoization already disabled"); - Preconditions.checkState(serializer == null, "serializer already present"); - return new SerializationContext( - registry, dependencies, MemoizationPermission.DISABLED, serializer); + // TODO(shahan): consider making codedOut a member of this class. + public void serialize(Object object, CodedOutputStream codedOut) + throws IOException, SerializationException { + ObjectCodecRegistry.CodecDescriptor descriptor = + recordAndGetDescriptorIfNotConstantOrNull(object, codedOut); + if (descriptor != null) { + if (serializer == null) { + descriptor.serialize(this, object, codedOut); + } else { + @SuppressWarnings("unchecked") + ObjectCodec<Object> castCodec = (ObjectCodec<Object>) descriptor.getCodec(); + serializer.serialize(this, object, castCodec, codedOut); + } + } + } + + @SuppressWarnings("unchecked") + public <T> T getDependency(Class<T> type) { + Preconditions.checkNotNull(type); + return (T) dependencies.get(type); } /** @@ -78,16 +86,16 @@ public class SerializationContext { * * <p>This method is idempotent: calling it on an already memoizing context will return the same * context. + * + * <p><em>This is public for testing and {@link + * com.google.devtools.build.lib.packages.PackageSerializer} only.</em> */ @CheckReturnValue public SerializationContext getMemoizingContext() { - Preconditions.checkState( - memoizationPermission == MemoizationPermission.ALLOWED, "memoization disabled"); if (serializer != null) { return this; } - return new SerializationContext( - this.registry, this.dependencies, memoizationPermission, new Memoizer.Serializer()); + return new SerializationContext(this.registry, this.dependencies, new Memoizer.Serializer()); } private boolean writeNullOrConstant(@Nullable Object object, CodedOutputStream codedOut) @@ -114,26 +122,4 @@ public class SerializationContext { codedOut.writeSInt32NoTag(descriptor.getTag()); return descriptor; } - - // TODO(shahan): consider making codedOut a member of this class. - public void serialize(Object object, CodedOutputStream codedOut) - throws IOException, SerializationException { - ObjectCodecRegistry.CodecDescriptor descriptor = - recordAndGetDescriptorIfNotConstantOrNull(object, codedOut); - if (descriptor != null) { - if (serializer == null) { - descriptor.serialize(this, object, codedOut); - } else { - @SuppressWarnings("unchecked") - ObjectCodec<Object> castCodec = (ObjectCodec<Object>) descriptor.getCodec(); - serializer.serialize(this, object, castCodec, codedOut); - } - } - } - - @SuppressWarnings("unchecked") - public <T> T getDependency(Class<T> type) { - Preconditions.checkNotNull(type); - return (T) dependencies.get(type); - } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java index 6901b9217d..302f1e9a49 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodec.java @@ -35,7 +35,7 @@ import java.lang.annotation.Target; * * <p>If applied to a field (which must be static and final), the field is stored as a "constant" * allowing for trivial serialization of it as an integer tag (see {@code CodecScanner} and - * {@code ObjectCodecRegistery}). In order to do that, a trivial associated "RegisteredSingleton" + * {@code ObjectCodecRegistry}). In order to do that, a trivial associated "RegisteredSingleton" * class is generated. */ @Target({ElementType.TYPE, ElementType.FIELD}) @@ -89,20 +89,6 @@ public @interface AutoCodec { Strategy strategy() default Strategy.INSTANTIATOR; - /** Whether to start memoizing values below this codec. */ - enum Memoization { - /** Do not start memoization, but also do not disable memoization if it is already happening. */ - UNCHANGED, - /** - * Start memoizing. Memoization is assumed to always need a Skylark "Mutability" object. If this - * package does not have access to the {@link com.google.devtools.build.lib.syntax.Mutability} - * class, memoization cannot be started here. - */ - START_MEMOIZING - } - - Memoization memoization() default Memoization.UNCHANGED; - /** * Signals that the annotated element is only visible for use by serialization. It should not be * used by other callers. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java index fb945a93b7..395519d279 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecProcessor.java @@ -25,7 +25,6 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.skyframe.serialization.CodecScanningConstants; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; import com.google.devtools.build.lib.skyframe.serialization.SerializationException; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Memoization; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationCodeGenerator.Marshaller; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.FieldSpec; @@ -104,16 +103,15 @@ public class AutoCodecProcessor extends AbstractProcessor { if (element instanceof TypeElement) { TypeElement encodedType = (TypeElement) element; TypeSpec.Builder codecClassBuilder; - boolean startMemoizing = annotation.memoization() == Memoization.START_MEMOIZING; switch (annotation.strategy()) { case INSTANTIATOR: - codecClassBuilder = buildClassWithInstantiatorStrategy(encodedType, startMemoizing); + codecClassBuilder = buildClassWithInstantiatorStrategy(encodedType); break; case PUBLIC_FIELDS: - codecClassBuilder = buildClassWithPublicFieldsStrategy(encodedType, startMemoizing); + codecClassBuilder = buildClassWithPublicFieldsStrategy(encodedType); break; case AUTO_VALUE_BUILDER: - codecClassBuilder = buildClassWithAutoValueBuilderStrategy(encodedType, startMemoizing); + codecClassBuilder = buildClassWithAutoValueBuilderStrategy(encodedType); break; default: throw new IllegalArgumentException("Unknown strategy: " + annotation.strategy()); @@ -173,8 +171,7 @@ public class AutoCodecProcessor extends AbstractProcessor { .build(); } - private TypeSpec.Builder buildClassWithInstantiatorStrategy( - TypeElement encodedType, boolean startMemoizing) { + private TypeSpec.Builder buildClassWithInstantiatorStrategy(TypeElement encodedType) { ExecutableElement constructor = selectInstantiator(encodedType); List<? extends VariableElement> fields = constructor.getParameters(); @@ -183,15 +180,14 @@ public class AutoCodecProcessor extends AbstractProcessor { if (encodedType.getAnnotation(AutoValue.class) == null) { initializeUnsafeOffsets(codecClassBuilder, encodedType, fields); - codecClassBuilder.addMethod( - buildSerializeMethodWithInstantiator(encodedType, fields, startMemoizing)); + codecClassBuilder.addMethod(buildSerializeMethodWithInstantiator(encodedType, fields)); } else { codecClassBuilder.addMethod( - buildSerializeMethodWithInstantiatorForAutoValue(encodedType, fields, startMemoizing)); + buildSerializeMethodWithInstantiatorForAutoValue(encodedType, fields)); } MethodSpec.Builder deserializeBuilder = - AutoCodecUtil.initializeDeserializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeDeserializeMethodBuilder(encodedType, env); buildDeserializeBody(deserializeBuilder, fields); addReturnNew(deserializeBuilder, encodedType, constructor, /*builderVar=*/ null, env); codecClassBuilder.addMethod(deserializeBuilder.build()); @@ -199,8 +195,7 @@ public class AutoCodecProcessor extends AbstractProcessor { return codecClassBuilder; } - private TypeSpec.Builder buildClassWithAutoValueBuilderStrategy( - TypeElement encodedType, boolean startMemoizing) { + private TypeSpec.Builder buildClassWithAutoValueBuilderStrategy(TypeElement encodedType) { TypeElement builderType = findBuilderType(encodedType); List<ExecutableElement> getters = findGettersFromType(encodedType, builderType); ExecutableElement builderCreationMethod = findBuilderCreationMethod(encodedType, builderType); @@ -208,7 +203,7 @@ public class AutoCodecProcessor extends AbstractProcessor { TypeSpec.Builder codecClassBuilder = AutoCodecUtil.initializeCodecClassBuilder(encodedType, env); MethodSpec.Builder serializeBuilder = - AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, env); for (ExecutableElement getter : getters) { marshallers.writeSerializationCode( new Marshaller.Context( @@ -218,7 +213,7 @@ public class AutoCodecProcessor extends AbstractProcessor { } codecClassBuilder.addMethod(serializeBuilder.build()); MethodSpec.Builder deserializeBuilder = - AutoCodecUtil.initializeDeserializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeDeserializeMethodBuilder(encodedType, env); String builderVarName = buildDeserializeBodyWithBuilder( encodedType, builderType, deserializeBuilder, getters, builderCreationMethod); @@ -519,9 +514,9 @@ public class AutoCodecProcessor extends AbstractProcessor { } private MethodSpec buildSerializeMethodWithInstantiator( - TypeElement encodedType, List<? extends VariableElement> fields, boolean startMemoizing) { + TypeElement encodedType, List<? extends VariableElement> fields) { MethodSpec.Builder serializeBuilder = - AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, env); for (VariableElement parameter : fields) { Optional<FieldValueAndClass> hasField = getFieldByNameRecursive(encodedType, parameter.getSimpleName().toString()); @@ -624,17 +619,16 @@ public class AutoCodecProcessor extends AbstractProcessor { } private MethodSpec buildSerializeMethodWithInstantiatorForAutoValue( - TypeElement encodedType, List<? extends VariableElement> fields, boolean startMemoizing) { + TypeElement encodedType, List<? extends VariableElement> fields) { MethodSpec.Builder serializeBuilder = - AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, env); for (VariableElement parameter : fields) { addSerializeParameterWithGetter(encodedType, parameter, serializeBuilder); } return serializeBuilder.build(); } - private TypeSpec.Builder buildClassWithPublicFieldsStrategy( - TypeElement encodedType, boolean startMemoizing) { + private TypeSpec.Builder buildClassWithPublicFieldsStrategy(TypeElement encodedType) { TypeSpec.Builder codecClassBuilder = AutoCodecUtil.initializeCodecClassBuilder(encodedType, env); ImmutableList<? extends VariableElement> publicFields = @@ -642,10 +636,9 @@ public class AutoCodecProcessor extends AbstractProcessor { .stream() .filter(this::isPublicField) .collect(toImmutableList()); - codecClassBuilder.addMethod( - buildSerializeMethodWithPublicFields(encodedType, publicFields, startMemoizing)); + codecClassBuilder.addMethod(buildSerializeMethodWithPublicFields(encodedType, publicFields)); MethodSpec.Builder deserializeBuilder = - AutoCodecUtil.initializeDeserializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeDeserializeMethodBuilder(encodedType, env); buildDeserializeBody(deserializeBuilder, publicFields); addInstantiatePopulateFieldsAndReturn(deserializeBuilder, encodedType, publicFields); codecClassBuilder.addMethod(deserializeBuilder.build()); @@ -661,9 +654,9 @@ public class AutoCodecProcessor extends AbstractProcessor { } private MethodSpec buildSerializeMethodWithPublicFields( - TypeElement encodedType, List<? extends VariableElement> fields, boolean startMemoizing) { + TypeElement encodedType, List<? extends VariableElement> fields) { MethodSpec.Builder serializeBuilder = - AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, startMemoizing, env); + AutoCodecUtil.initializeSerializeMethodBuilder(encodedType, env); for (VariableElement parameter : fields) { String paramAccessor = "input." + parameter.getSimpleName(); marshallers.writeSerializationCode( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecUtil.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecUtil.java index 2ba6d9b2c2..34925eecb9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecUtil.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec/AutoCodecUtil.java @@ -69,10 +69,9 @@ class AutoCodecUtil { * Initializes the deserialize method. * * @param encodedType type being serialized - * @param startMemoizing whether memoization should start in this method. */ static MethodSpec.Builder initializeSerializeMethodBuilder( - TypeElement encodedType, boolean startMemoizing, ProcessingEnvironment env) { + TypeElement encodedType, ProcessingEnvironment env) { MethodSpec.Builder builder = MethodSpec.methodBuilder("serialize") .addModifiers(Modifier.PUBLIC) @@ -83,9 +82,6 @@ class AutoCodecUtil { .addParameter(SerializationContext.class, "context") .addParameter(TypeName.get(env.getTypeUtils().erasure(encodedType.asType())), "input") .addParameter(CodedOutputStream.class, "codedOut"); - if (startMemoizing) { - builder.addStatement("context = context.getMemoizingContext()"); - } return builder; } @@ -93,10 +89,9 @@ class AutoCodecUtil { * Initializes the deserialize method. * * @param encodedType type being serialized - * @param startMemoizing whether memoization should start in this method. */ static MethodSpec.Builder initializeDeserializeMethodBuilder( - TypeElement encodedType, boolean startMemoizing, ProcessingEnvironment env) { + TypeElement encodedType, ProcessingEnvironment env) { MethodSpec.Builder builder = MethodSpec.methodBuilder("deserialize") .addModifiers(Modifier.PUBLIC) @@ -106,9 +101,6 @@ class AutoCodecUtil { .addException(IOException.class) .addParameter(DeserializationContext.class, "context") .addParameter(CodedInputStream.class, "codedIn"); - if (startMemoizing) { - builder.addStatement("context = context.getMemoizingContext()"); - } return builder; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java index 91926dbc8e..b45d443f6c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils/SerializationTester.java @@ -22,13 +22,11 @@ import com.google.common.base.Stopwatch; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.skyframe.serialization.AutoRegistry; -import com.google.devtools.build.lib.skyframe.serialization.DeserializationContext; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecRegistry; import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs; import com.google.devtools.build.lib.skyframe.serialization.SerializationException; import com.google.protobuf.ByteString; -import com.google.protobuf.CodedOutputStream; import com.google.protobuf.InvalidProtocolBufferException; import java.io.IOException; import java.util.ArrayList; @@ -126,24 +124,20 @@ public class SerializationTester { private ByteString serialize(Object subject, ObjectCodecs codecs) throws SerializationException, IOException { - if (!memoize) { + if (memoize) { + return codecs.serializeMemoized(subject); + } else { return codecs.serialize(subject); } - ByteString.Output output = ByteString.newOutput(); - CodedOutputStream codedOut = CodedOutputStream.newInstance(output); - codecs.getSerializationContextForTesting().getMemoizingContext().serialize(subject, codedOut); - codedOut.flush(); - return output.toByteString(); } private Object deserialize(ByteString serialized, ObjectCodecs codecs) throws SerializationException, IOException { - if (!memoize) { + if (memoize) { + return codecs.deserializeMemoized(serialized); + } else { return codecs.deserialize(serialized); } - DeserializationContext context = - codecs.getDeserializationContextForTesting().getMemoizingContext(); - return context.deserialize(serialized.newCodedInput()); } /** Runs serialization/deserialization tests. */ diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 697c317825..46253faa49 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.Memoization; import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; import com.google.devtools.build.lib.syntax.Mutability.Freezable; import com.google.devtools.build.lib.syntax.Mutability.MutabilityException; @@ -478,7 +477,7 @@ public final class Environment implements Freezable { @Immutable // TODO(janakr,brandjon): Do Extensions actually have to start their own memoization? Or can we // have a node higher up in the hierarchy inject the mutability? - @AutoCodec(memoization = Memoization.START_MEMOIZING) + @AutoCodec public static final class Extension { private final ImmutableMap<String, Object> bindings; |