From 192c8904999a3e4407999a12c85fc4e9114b5e97 Mon Sep 17 00:00:00 2001 From: janakr Date: Fri, 19 Jan 2018 22:29:42 -0800 Subject: Start serializing ArtifactOwner: put in a simple codec for the null artifact owner and fix up BuildConfigurationValue.Key. ConfiguredTargetKey is going to need some modifications to AutoCodec: probably the long-awaited static "create" method. PiperOrigin-RevId: 182630181 --- .../devtools/build/lib/actions/ActionRegistry.java | 16 ------- .../devtools/build/lib/actions/Artifact.java | 15 ++++--- .../build/lib/actions/ArtifactFactory.java | 4 +- .../devtools/build/lib/actions/ArtifactOwner.java | 40 +++++++++++------ .../com/google/devtools/build/lib/actions/BUILD | 2 + .../lib/skyframe/BuildConfigurationValue.java | 52 ++++++++++++++++++++-- .../build/lib/skyframe/ConfiguredTargetKey.java | 18 ++++---- 7 files changed, 98 insertions(+), 49 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java b/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java index de85ce9f80..df064e8fb1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionRegistry.java @@ -14,8 +14,6 @@ package com.google.devtools.build.lib.actions; -import com.google.common.annotations.VisibleForTesting; - /** * An interface for registering actions. */ @@ -30,18 +28,4 @@ public interface ActionRegistry { * these actions. */ ArtifactOwner getOwner(); - - /** - * An action registry that does exactly nothing. - */ - @VisibleForTesting - public static final ActionRegistry NOP = new ActionRegistry() { - @Override - public void registerAction(ActionAnalysisMetadata... actions) {} - - @Override - public ArtifactOwner getOwner() { - return ArtifactOwner.NULL_OWNER; - } - }; } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java index eb9165a55c..e371ec1670 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java @@ -238,7 +238,7 @@ public class Artifact */ @VisibleForTesting public Artifact(Path path, ArtifactRoot root, PathFragment execPath) { - this(path, root, execPath, ArtifactOwner.NULL_OWNER); + this(path, root, execPath, ArtifactOwner.NullArtifactOwner.INSTANCE); } /** @@ -251,7 +251,7 @@ public class Artifact path, root, root.getExecPath().getRelative(root.getRoot().relativize(path)), - ArtifactOwner.NULL_OWNER); + ArtifactOwner.NullArtifactOwner.INSTANCE); } /** Constructs a source or derived Artifact for the specified root-relative path and root. */ @@ -261,7 +261,7 @@ public class Artifact root.getRoot().getRelative(rootRelativePath), root, root.getExecPath().getRelative(rootRelativePath), - ArtifactOwner.NULL_OWNER); + ArtifactOwner.NullArtifactOwner.INSTANCE); } public final Path getPath() { @@ -337,10 +337,11 @@ public class Artifact /** * Gets the {@code ActionLookupKey} of the {@code ConfiguredTarget} that owns this artifact, if it - * was set. Otherwise, this should be a dummy value -- either {@link ArtifactOwner#NULL_OWNER} or - * a dummy owner set in tests. Such a dummy value should only occur for source artifacts if - * created without specifying the owner, or for special derived artifacts, such as target - * completion middleman artifacts, build info artifacts, and the like. + * was set. Otherwise, this should be a dummy value -- either {@link + * ArtifactOwner.NullArtifactOwner#INSTANCE} or a dummy owner set in tests. Such a dummy value + * should only occur for source artifacts if created without specifying the owner, or for special + * derived artifacts, such as target completion middleman artifacts, build info artifacts, and the + * like. */ public final ArtifactOwner getArtifactOwner() { return owner; diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java index 7382944cc3..7a0f88b7b4 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java @@ -155,7 +155,7 @@ public class ArtifactFactory implements ArtifactResolver { @Override public Artifact getSourceArtifact(PathFragment execPath, ArtifactRoot root) { - return getSourceArtifact(execPath, root, ArtifactOwner.NULL_OWNER); + return getSourceArtifact(execPath, root, ArtifactOwner.NullArtifactOwner.INSTANCE); } private void validatePath(PathFragment rootRelativePath, ArtifactRoot root) { @@ -424,7 +424,7 @@ public class ArtifactFactory implements ArtifactResolver { sourceArtifactCache.markEntryAsValid(execPath); } else { // Must be a new artifact or artifact in the cache is stale, so create a new one. - artifact = getSourceArtifact(execPath, sourceRoot, ArtifactOwner.NULL_OWNER); + artifact = getSourceArtifact(execPath, sourceRoot, ArtifactOwner.NullArtifactOwner.INSTANCE); } return artifact; } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactOwner.java index 597893f6b5..02904d45bc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactOwner.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactOwner.java @@ -15,26 +15,40 @@ package com.google.devtools.build.lib.actions; import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.skyframe.serialization.ObjectCodec; +import com.google.devtools.build.lib.skyframe.serialization.SingletonCodec; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; /** * An interface for {@code ActionLookupKey}, or at least for a {@link Label}. Only tests and * internal {@link Artifact}-generators should implement this interface -- otherwise, {@code * ActionLookupKey} and its subclasses should be the only implementation. */ +@AutoCodec(strategy = AutoCodec.Strategy.POLYMORPHIC) public interface ArtifactOwner { + ObjectCodec CODEC = new ArtifactOwner_AutoCodec(); + Label getLabel(); - @VisibleForTesting - ArtifactOwner NULL_OWNER = - new ArtifactOwner() { - @Override - public Label getLabel() { - return null; - } - - @Override - public String toString() { - return "NULL_OWNER"; - } - }; + /** + * An {@link ArtifactOwner} that just returns null for its label. Only for use with resolved + * source artifacts and tests. + */ + class NullArtifactOwner implements ArtifactOwner { + @VisibleForTesting public static final NullArtifactOwner INSTANCE = new NullArtifactOwner(); + + static final ObjectCodec CODEC = SingletonCodec.of(INSTANCE, "NULL_OWNER"); + + private NullArtifactOwner() {} + + @Override + public Label getLabel() { + return null; + } + + @Override + public String toString() { + return "NULL_OWNER"; + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 608ea70b8d..2c3f3abb0e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -33,6 +33,8 @@ java_library( "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/shell", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java index 46e616c17f..c42539aace 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildConfigurationValue.java @@ -22,11 +22,17 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.skyframe.serialization.InjectingObjectCodec; +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; +import com.google.devtools.build.lib.skyframe.serialization.strings.StringCodecs; import com.google.devtools.build.lib.vfs.FileSystemProvider; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import com.google.protobuf.CodedInputStream; +import com.google.protobuf.CodedOutputStream; +import java.io.IOException; import java.io.Serializable; import java.util.Objects; import java.util.Set; @@ -59,8 +65,8 @@ public class BuildConfigurationValue implements SkyValue { * @param buildOptions the build options the fragments should be built from */ @ThreadSafe - public static SkyKey key(Set> fragments, - BuildOptions buildOptions) { + public static Key key( + Set> fragments, BuildOptions buildOptions) { return keyInterner.intern( new Key( ImmutableSortedSet.copyOf(BuildConfiguration.lexicalFragmentSorter, fragments), @@ -68,12 +74,15 @@ public class BuildConfigurationValue implements SkyValue { } static final class Key implements SkyKey, Serializable { + static final ObjectCodec CODEC = new Codec(); + private final ImmutableSortedSet> fragments; private final BuildOptions buildOptions; // If hashCode really is -1, we'll recompute it from scratch each time. Oh well. private volatile int hashCode = -1; - Key(ImmutableSortedSet> fragments, BuildOptions buildOptions) { + private Key( + ImmutableSortedSet> fragments, BuildOptions buildOptions) { this.fragments = fragments; this.buildOptions = Preconditions.checkNotNull(buildOptions); } @@ -111,5 +120,42 @@ public class BuildConfigurationValue implements SkyValue { } return hashCode; } + + private static class Codec implements ObjectCodec { + @Override + public Class getEncodedClass() { + return Key.class; + } + + @Override + public void serialize(Key obj, CodedOutputStream codedOut) + throws SerializationException, IOException { + BuildOptions.CODEC.serialize(obj.buildOptions, codedOut); + codedOut.writeInt32NoTag(obj.fragments.size()); + for (Class fragment : obj.fragments) { + StringCodecs.asciiOptimized().serialize(fragment.getName(), codedOut); + } + } + + @Override + @SuppressWarnings("unchecked") // Class cast + public Key deserialize(CodedInputStream codedIn) throws SerializationException, IOException { + BuildOptions buildOptions = BuildOptions.CODEC.deserialize(codedIn); + int fragmentsSize = codedIn.readInt32(); + ImmutableSortedSet.Builder> fragmentsBuilder = + ImmutableSortedSet.orderedBy(BuildConfiguration.lexicalFragmentSorter); + for (int i = 0; i < fragmentsSize; i++) { + try { + fragmentsBuilder.add( + (Class) + Class.forName(StringCodecs.asciiOptimized().deserialize(codedIn))); + } catch (ClassNotFoundException e) { + throw new SerializationException( + "Couldn't deserialize BuildConfigurationValue$Key fragment class", e); + } + } + return key(fragmentsBuilder.build(), buildOptions); + } + } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java index b69193fca1..8e8baa5226 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java @@ -23,7 +23,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.skyframe.SkyFunctionName; -import com.google.devtools.build.skyframe.SkyKey; import java.util.Objects; import javax.annotation.Nullable; @@ -33,11 +32,11 @@ import javax.annotation.Nullable; */ public class ConfiguredTargetKey extends ActionLookupKey { private final Label label; - @Nullable private final SkyKey configurationKey; + @Nullable private final BuildConfigurationValue.Key configurationKey; private transient int hashCode; - private ConfiguredTargetKey(Label label, @Nullable SkyKey configurationKey) { + private ConfiguredTargetKey(Label label, @Nullable BuildConfigurationValue.Key configurationKey) { this.label = Preconditions.checkNotNull(label); this.configurationKey = configurationKey; } @@ -58,11 +57,11 @@ public class ConfiguredTargetKey extends ActionLookupKey { BlazeInterners.newWeakInterner(); public static ConfiguredTargetKey of(Label label, @Nullable BuildConfiguration configuration) { - SkyKey configurationKey = + BuildConfigurationValue.Key configurationKey = configuration == null ? null : BuildConfigurationValue.key( - configuration.fragmentClasses(), configuration.getOptions()); + configuration.fragmentClasses(), configuration.getOptions()); return of( label, configurationKey, @@ -70,7 +69,9 @@ public class ConfiguredTargetKey extends ActionLookupKey { } static ConfiguredTargetKey of( - Label label, @Nullable SkyKey configurationKey, boolean isHostConfiguration) { + Label label, + @Nullable BuildConfigurationValue.Key configurationKey, + boolean isHostConfiguration) { if (isHostConfiguration) { return hostInterner.intern(new HostConfiguredTargetKey(label, configurationKey)); } else { @@ -89,7 +90,7 @@ public class ConfiguredTargetKey extends ActionLookupKey { } @Nullable - SkyKey getConfigurationKey() { + BuildConfigurationValue.Key getConfigurationKey() { return configurationKey; } @@ -163,7 +164,8 @@ public class ConfiguredTargetKey extends ActionLookupKey { } private static class HostConfiguredTargetKey extends ConfiguredTargetKey { - private HostConfiguredTargetKey(Label label, @Nullable SkyKey configurationKey) { + private HostConfiguredTargetKey( + Label label, @Nullable BuildConfigurationValue.Key configurationKey) { super(label, configurationKey); } -- cgit v1.2.3