PiperOrigin-RevId: 206181674

1 files changed, 33 insertions, 191 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
index e4be9a3f2c..81c3d9b3a4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
@@ -27,10 +27,6 @@ import com.google.common.collect.Multimap;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
-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.SerializationContext;
-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.util.Fingerprint;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -40,10 +36,6 @@ import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsClassProvider;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
-import com.google.protobuf.ByteString;
-import com.google.protobuf.CodedInputStream;
-import com.google.protobuf.CodedOutputStream;
-import java.io.IOException;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -58,8 +50,6 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 
@@ -70,7 +60,6 @@ import javax.annotation.Nullable;
 public final class BuildOptions implements Cloneable, Serializable {
   private static final Comparator<Class<? extends FragmentOptions>>
       lexicalFragmentOptionsComparator = Comparator.comparing(Class::getName);
-  private static final Logger logger = Logger.getLogger(BuildOptions.class.getName());
 
   /**
    * Creates a BuildOptions object with all options set to their default values, processed by the
@@ -92,9 +81,7 @@ public final class BuildOptions implements Cloneable, Serializable {
     return optionsParser;
   }
 
-  /**
-   * Creates a new BuildOptions instance for host.
-   */
+  /** Creates a new BuildOptions instance for host. */
   public BuildOptions createHostOptions() {
     Builder builder = builder();
     for (FragmentOptions options : fragmentOptionsMap.values()) {
@@ -104,8 +91,8 @@ public final class BuildOptions implements Cloneable, Serializable {
   }
 
   /**
-   * Returns an equivalent instance to this one with only options from the given
-   * {@link FragmentOptions} classes.
+   * Returns an equivalent instance to this one with only options from the given {@link
+   * FragmentOptions} classes.
    */
   public BuildOptions trim(Set<Class<? extends FragmentOptions>> optionsClasses) {
     Builder builder = builder();
@@ -121,8 +108,8 @@ public final class BuildOptions implements Cloneable, Serializable {
   }
 
   /**
-   * Creates a BuildOptions class by taking the option values from an options provider
-   * (eg. an OptionsParser).
+   * Creates a BuildOptions class by taking the option values from an options provider (eg. an
+   * OptionsParser).
    */
   public static BuildOptions of(
       Iterable<Class<? extends FragmentOptions>> optionsList, OptionsClassProvider provider) {
@@ -133,15 +120,14 @@ public final class BuildOptions implements Cloneable, Serializable {
     return builder.build();
   }
 
-  /**
-   * Creates a BuildOptions class by taking the option values from command-line arguments.
-   */
+  /** Creates a BuildOptions class by taking the option values from command-line arguments. */
   @VisibleForTesting
-  public static BuildOptions of(List<Class<?extends FragmentOptions>> optionsList, String... args)
+  public static BuildOptions of(List<Class<? extends FragmentOptions>> optionsList, String... args)
       throws OptionsParsingException {
     Builder builder = builder();
-    OptionsParser parser = OptionsParser.newOptionsParser(
-        ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsList));
+    OptionsParser parser =
+        OptionsParser.newOptionsParser(
+            ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsList));
     parser.parse(args);
     for (Class<? extends FragmentOptions> optionsClass : optionsList) {
       builder.add(parser.getOptions(optionsClass));
@@ -149,18 +135,14 @@ public final class BuildOptions implements Cloneable, Serializable {
     return builder.build();
   }
 
-  /**
-   * Returns the actual instance of a FragmentOptions class.
-   */
+  /** Returns the actual instance of a FragmentOptions class. */
   public <T extends FragmentOptions> T get(Class<T> optionsClass) {
     FragmentOptions options = fragmentOptionsMap.get(optionsClass);
     Preconditions.checkNotNull(options, "fragment options unavailable: " + optionsClass.getName());
     return optionsClass.cast(options);
   }
 
-  /**
-   * Returns true if these options contain the given {@link FragmentOptions}.
-   */
+  /** Returns true if these options contain the given {@link FragmentOptions}. */
   public boolean contains(Class<? extends FragmentOptions> optionsClass) {
     return fragmentOptionsMap.containsKey(optionsClass);
   }
@@ -168,7 +150,7 @@ public final class BuildOptions implements Cloneable, Serializable {
   // It would be very convenient to use a Multimap here, but we cannot do that because we need to
   // support defaults labels that have zero elements.
   ImmutableMap<String, ImmutableSet<Label>> getDefaultsLabels() {
-    Map<String, Set<Label>> collector  = new TreeMap<>();
+    Map<String, Set<Label>> collector = new TreeMap<>();
     for (FragmentOptions fragment : fragmentOptionsMap.values()) {
       for (Map.Entry<String, Set<Label>> entry : fragment.getDefaultsLabels().entrySet()) {
         if (!collector.containsKey(entry.getKey())) {
@@ -186,9 +168,7 @@ public final class BuildOptions implements Cloneable, Serializable {
     return result.build();
   }
 
-  /**
-   * The cache key for the options collection. Recomputes cache key every time it's called.
-   */
+  /** The cache key for the options collection. Recomputes cache key every time it's called. */
   public String computeCacheKey() {
     StringBuilder keyBuilder = new StringBuilder();
     for (FragmentOptions options : fragmentOptionsMap.values()) {
@@ -201,9 +181,7 @@ public final class BuildOptions implements Cloneable, Serializable {
     return Fingerprint.md5Digest(computeCacheKey());
   }
 
-  /**
-   * String representation of build options.
-   */
+  /** String representation of build options. */
   @Override
   public String toString() {
     StringBuilder stringBuilder = new StringBuilder();
@@ -213,16 +191,12 @@ public final class BuildOptions implements Cloneable, Serializable {
     return stringBuilder.toString();
   }
 
-  /**
-   * Returns the options contained in this collection.
-   */
+  /** Returns the options contained in this collection. */
   public Collection<FragmentOptions> getOptions() {
     return fragmentOptionsMap.values();
   }
 
-  /**
-   * Creates a copy of the BuildOptions object that contains copies of the FragmentOptions.
-   */
+  /** Creates a copy of the BuildOptions object that contains copies of the FragmentOptions. */
   @Override
   public BuildOptions clone() {
     ImmutableMap.Builder<Class<? extends FragmentOptions>, FragmentOptions> builder =
@@ -287,9 +261,7 @@ public final class BuildOptions implements Cloneable, Serializable {
   @Nullable private volatile byte[] fingerprint;
   private volatile int hashCode;
 
-  /**
-   * Maps options class definitions to FragmentOptions objects.
-   */
+  /** Maps options class definitions to FragmentOptions objects. */
   private final ImmutableMap<Class<? extends FragmentOptions>, FragmentOptions> fragmentOptionsMap;
 
   @AutoCodec.VisibleForSerialization
@@ -318,20 +290,16 @@ public final class BuildOptions implements Cloneable, Serializable {
     return builder.build();
   }
 
-  /**
-   * Creates a builder object for BuildOptions
-   */
+  /** Creates a builder object for BuildOptions */
   public static Builder builder() {
     return new Builder();
   }
 
-  /**
-   * Builder class for BuildOptions.
-   */
+  /** Builder class for BuildOptions. */
   public static class Builder {
     /**
-     * Adds a new FragmentOptions instance to the builder. Overrides previous instances of the
-     * exact same subclass of FragmentOptions.
+     * Adds a new FragmentOptions instance to the builder. Overrides previous instances of the exact
+     * same subclass of FragmentOptions.
      */
     public <T extends FragmentOptions> Builder add(T options) {
       builderMap.put(options.getClass(), options);
@@ -377,12 +345,14 @@ public final class BuildOptions implements Cloneable, Serializable {
     // Check and report if either class has been trimmed of an options class that exists in the
     // other.
     ImmutableSet<Class<? extends FragmentOptions>> firstOptionClasses =
-        first.getOptions()
+        first
+            .getOptions()
             .stream()
             .map(FragmentOptions::getClass)
             .collect(ImmutableSet.toImmutableSet());
     ImmutableSet<Class<? extends FragmentOptions>> secondOptionClasses =
-        second.getOptions()
+        second
+            .getOptions()
             .stream()
             .map(FragmentOptions::getClass)
             .collect(ImmutableSet.toImmutableSet());
@@ -470,7 +440,7 @@ public final class BuildOptions implements Cloneable, Serializable {
    * A diff class for BuildOptions. Fields are meant to be populated and returned by {@link
    * BuildOptions#diff}
    */
-  public static class OptionsDiff{
+  public static class OptionsDiff {
     private final Multimap<Class<? extends FragmentOptions>, OptionDefinition> differingOptions =
         ArrayListMultimap.create();
     // The keyset for the {@link first} and {@link second} maps are identical and indicate which
@@ -542,10 +512,9 @@ public final class BuildOptions implements Cloneable, Serializable {
 
     public List<String> getPrettyPrintList() {
       List<String> toReturn = new ArrayList<>();
-        first.forEach(
-            (option, value) ->
-                toReturn.add(
-                    option.getOptionName() + ":" + value + " -> " + second.get(option)));
+      first.forEach(
+          (option, value) ->
+              toReturn.add(option.getOptionName() + ":" + value + " -> " + second.get(option)));
       return toReturn;
     }
   }
@@ -555,6 +524,7 @@ public final class BuildOptions implements Cloneable, Serializable {
    * another: the full fragments of the second one, the fragment classes of the first that should be
    * omitted, and the values of any fields that should be changed.
    */
+  @AutoCodec
   public static class OptionsDiffForReconstruction {
     private final Map<Class<? extends FragmentOptions>, Map<String, Object>> differingOptions;
     private final ImmutableSet<Class<? extends FragmentOptions>> extraFirstFragmentClasses;
@@ -630,141 +600,13 @@ public final class BuildOptions implements Cloneable, Serializable {
       return MoreObjects.toStringHelper(this)
           .add("differingOptions", differingOptions)
           .add("extraFirstFragmentClasses", extraFirstFragmentClasses)
-          .add("extraSecondFragments", extraSecondFragments).toString();
+          .add("extraSecondFragments", extraSecondFragments)
+          .toString();
     }
 
     @Override
     public int hashCode() {
       return 31 * Arrays.hashCode(baseFingerprint) + checksum.hashCode();
     }
-
-    private static class Codec implements ObjectCodec<OptionsDiffForReconstruction> {
-
-      @Override
-      public Class<OptionsDiffForReconstruction> getEncodedClass() {
-        return OptionsDiffForReconstruction.class;
-      }
-
-      @Override
-      public void serialize(
-          SerializationContext context,
-          OptionsDiffForReconstruction diff,
-          CodedOutputStream codedOut)
-          throws SerializationException, IOException {
-        OptionsDiffCache cache = context.getDependency(OptionsDiffCache.class);
-        ByteString bytes = cache.getBytesFromOptionsDiff(diff);
-        if (bytes == null) {
-          context = context.getNewNonMemoizingContext();
-          ByteString.Output byteStringOut = ByteString.newOutput();
-          CodedOutputStream bytesOut = CodedOutputStream.newInstance(byteStringOut);
-          context.serialize(diff.differingOptions, bytesOut);
-          context.serialize(diff.extraFirstFragmentClasses, bytesOut);
-          context.serialize(diff.extraSecondFragments, bytesOut);
-          bytesOut.writeByteArrayNoTag(diff.baseFingerprint);
-          context.serialize(diff.checksum, bytesOut);
-          bytesOut.flush();
-          byteStringOut.flush();
-          int optionsDiffSize = byteStringOut.size();
-          bytes = byteStringOut.toByteString();
-          cache.putBytesFromOptionsDiff(diff, bytes);
-          logger.info(
-              "Serialized OptionsDiffForReconstruction "
-                  + diff.toString()
-                  + ". Diff took "
-                  + optionsDiffSize
-                  + " bytes.");
-        }
-        codedOut.writeBytesNoTag(bytes);
-      }
-
-      @Override
-      public OptionsDiffForReconstruction deserialize(
-          DeserializationContext context, CodedInputStream codedIn)
-          throws SerializationException, IOException {
-        OptionsDiffCache cache = context.getDependency(OptionsDiffCache.class);
-        ByteString bytes = codedIn.readBytes();
-        OptionsDiffForReconstruction diff = cache.getOptionsDiffFromBytes(bytes);
-        if (diff == null) {
-          CodedInputStream codedInput = bytes.newCodedInput();
-          context = context.getNewNonMemoizingContext();
-          Map<Class<? extends FragmentOptions>, Map<String, Object>> differingOptions =
-              context.deserialize(codedInput);
-          ImmutableSet<Class<? extends FragmentOptions>> extraFirstFragmentClasses =
-              context.deserialize(codedInput);
-          ImmutableList<FragmentOptions> extraSecondFragments = context.deserialize(codedInput);
-          byte[] baseFingerprint = codedInput.readByteArray();
-          String checksum = context.deserialize(codedInput);
-          diff =
-              new OptionsDiffForReconstruction(
-                  differingOptions,
-                  extraFirstFragmentClasses,
-                  extraSecondFragments,
-                  baseFingerprint,
-                  checksum);
-          cache.putOptionsDiffFromBytes(bytes, diff);
-        }
-        return diff;
-      }
-    }
-  }
-
-  /**
-   * Injected cache for {@code Codec}, so that we don't have to repeatedly serialize the same
-   * object. We still incur the over-the-wire cost of the bytes, but we don't use CPU to repeatedly
-   * compute it.
-   *
-   * <p>We provide the cache as an injected dependency so that different serializers' caches are
-   * isolated.
-   */
-  public interface OptionsDiffCache {
-    ByteString getBytesFromOptionsDiff(OptionsDiffForReconstruction diff);
-
-    void putBytesFromOptionsDiff(OptionsDiffForReconstruction diff, ByteString bytes);
-
-    OptionsDiffForReconstruction getOptionsDiffFromBytes(ByteString bytes);
-
-    void putOptionsDiffFromBytes(ByteString bytes, OptionsDiffForReconstruction diff);
-  }
-
-  /**
-   * Implementation of the {@link OptionsDiffCache} that acts as a {@code BiMap} utilizing two
-   * {@code ConcurrentHashMaps}.
-   */
-  public static class DiffToByteCache implements OptionsDiffCache {
-    // We expect there to be very few elements so keeping the reverse map as well as the forward
-    // map should be very cheap.
-    private static final ConcurrentHashMap<OptionsDiffForReconstruction, ByteString>
-        diffToByteStringMap = new ConcurrentHashMap<>();
-    private static final ConcurrentHashMap<ByteString, OptionsDiffForReconstruction>
-        byteStringToDiffMap = new ConcurrentHashMap<>();
-
-    @VisibleForTesting
-    public DiffToByteCache() {}
-
-    @Override
-    public ByteString getBytesFromOptionsDiff(OptionsDiffForReconstruction diff) {
-      return diffToByteStringMap.get(diff);
-    }
-
-    @Override
-    public void putBytesFromOptionsDiff(OptionsDiffForReconstruction diff, ByteString bytes) {
-      // We need to insert data into map that will be used for deserialization first in case there
-      // is a race between two threads. This can occur when one thread has called this method
-      // and populates one map, giving the other thread a cache hit, but hasn't populated the
-      // other map yet so on deserialization there is a cache miss.
-      byteStringToDiffMap.put(bytes, diff);
-      diffToByteStringMap.put(diff, bytes);
-    }
-
-    @Override
-    public OptionsDiffForReconstruction getOptionsDiffFromBytes(ByteString bytes) {
-      return byteStringToDiffMap.get(bytes);
-    }
-
-    @Override
-    public void putOptionsDiffFromBytes(ByteString bytes, OptionsDiffForReconstruction diff) {
-      diffToByteStringMap.put(diff, bytes);
-      byteStringToDiffMap.put(bytes, diff);
-    }
   }
 }