Store the maximum source version encountered along with the "hash" of…

… the source versions. Not active for ActionSketchFunction because it uses content hashes, not version hashes. PiperOrigin-RevId: 371137349
Bencodes · Apr 29, 2021 · 21d1bee · 21d1bee
1 parent e4cc0b9
commit 21d1bee
Show file tree

Hide file tree

Showing 7 changed files with 168 additions and 18 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actionsketch/ActionSketch.java b/src/main/java/com/google/devtools/build/lib/actionsketch/ActionSketch.java
@@ -38,7 +38,7 @@ public abstract class ActionSketch implements SkyValue {
           .autoBuild();
 
   @Nullable
-  public abstract BigInteger transitiveSourceHash();
+  public abstract HashAndVersion transitiveSourceHash();
 
   @Nullable
   public abstract BigInteger transitiveActionLookupHash();
@@ -52,12 +52,12 @@ public static Builder builder() {
   /** A builder for {@link ActionSketch}. */
   @AutoValue.Builder
   public abstract static class Builder {
-    public abstract Builder setTransitiveSourceHash(BigInteger transitiveSourceHash);
+    public abstract Builder setTransitiveSourceHash(HashAndVersion transitiveSourceHash);
 
     public abstract Builder setTransitiveActionLookupHash(BigInteger transitiveActionLookupHash);
 
     @Nullable
-    abstract BigInteger transitiveSourceHash();
+    abstract HashAndVersion transitiveSourceHash();
 
     @Nullable
     abstract BigInteger transitiveActionLookupHash();
@@ -82,7 +82,16 @@ public final void writeTo(ByteBuffer buffer) {
     writeNextValue(transitiveActionLookupHash(), buffer);
   }
 
-  public static void writeNextValue(@Nullable BigInteger value, ByteBuffer buffer) {
+  private static void writeNextValue(HashAndVersion value, ByteBuffer buffer) {
+    if (value == null) {
+      buffer.put((byte) -1);
+    } else {
+      byte[] bytes = value.hash().toByteArray();
+      buffer.put((byte) bytes.length).put(bytes).putLong(value.version());
+    }
+  }
+
+  private static void writeNextValue(BigInteger value, ByteBuffer buffer) {
     if (value == null) {
       buffer.put((byte) -1);
     } else {
@@ -98,19 +107,31 @@ public static ActionSketch fromBytes(ByteString inputBytes) {
   public static ActionSketch fromByteBuffer(ByteBuffer buffer) {
     Builder builder =
         builder()
-            .setTransitiveSourceHash(readNextValue(buffer))
-            .setTransitiveActionLookupHash(readNextValue(buffer));
+            .setTransitiveSourceHash(readNextHashAndVersion(buffer))
+            .setTransitiveActionLookupHash(readNextBigInteger(buffer));
     return builder.build();
   }
 
   @Nullable
-  public static BigInteger readNextValue(ByteBuffer buffer) {
+  private static BigInteger readNextBigInteger(ByteBuffer buffer) {
+    byte length = buffer.get();
+    if (length < 0) {
+      return null;
+    }
+    byte[] val = new byte[length];
+    buffer.get(val);
+    return new BigInteger(1, val);
+  }
+
+  @Nullable
+  private static HashAndVersion readNextHashAndVersion(ByteBuffer buffer) {
     byte length = buffer.get();
     if (length < 0) {
       return null;
     }
     byte[] val = new byte[length];
     buffer.get(val);
-    return new BigInteger(val);
+    long version = buffer.getLong();
+    return HashAndVersion.create(new BigInteger(val), version);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/actionsketch/BUILD b/src/main/java/com/google/devtools/build/lib/actionsketch/BUILD
@@ -10,8 +10,12 @@ filegroup(
 
 java_library(
     name = "action_sketch",
-    srcs = glob(["*.java"]),
+    srcs = glob(
+        ["*.java"],
+        exclude = ["HashAndVersion.java"],
+    ),
     deps = [
+        ":hash_and_version",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
@@ -21,3 +25,14 @@ java_library(
         "//third_party/protobuf:protobuf_java",
     ],
 )
+
+java_library(
+    name = "hash_and_version",
+    srcs = ["HashAndVersion.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/util",
+        "//third_party:auto_value",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
diff --git a/src/main/java/com/google/devtools/build/lib/actionsketch/HashAndVersion.java b/src/main/java/com/google/devtools/build/lib/actionsketch/HashAndVersion.java
@@ -0,0 +1,56 @@
+// Copyright 2021 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.actionsketch;
+
+import static com.google.common.base.Preconditions.checkState;
+import static java.lang.Math.max;
+
+import com.google.auto.value.AutoValue;
+import com.google.devtools.build.lib.util.BigIntegerFingerprintUtils;
+import java.math.BigInteger;
+import javax.annotation.Nullable;
+
+/** Container holding a version and a {@link BigInteger} hash that has the version and more. */
+@AutoValue
+public abstract class HashAndVersion {
+  public static final HashAndVersion ZERO = create(BigInteger.ZERO, 0L);
+
+  public abstract BigInteger hash();
+
+  public abstract long version();
+
+  @Nullable
+  public static HashAndVersion create(BigInteger hash, long version) {
+    if (hash == null) {
+      checkState(version == Long.MAX_VALUE, "no hash with valid version %s", version);
+      return null;
+    }
+    return new AutoValue_HashAndVersion(hash, version);
+  }
+
+  public static HashAndVersion createNoVersion(BigInteger hash) {
+    return create(hash, 0L);
+  }
+
+  @Nullable
+  public static HashAndVersion composeNullable(HashAndVersion first, HashAndVersion second) {
+    if (first == null || second == null) {
+      return null;
+    }
+    return create(
+        BigIntegerFingerprintUtils.compose(first.hash(), second.hash()),
+        max(first.version(), second.version()));
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/actionsketch/Sketches.java b/src/main/java/com/google/devtools/build/lib/actionsketch/Sketches.java
@@ -14,13 +14,16 @@
 
 package com.google.devtools.build.lib.actionsketch;
 
+import static java.lang.Math.max;
+
 import com.google.common.hash.HashCode;
 import com.google.common.hash.Hasher;
 import com.google.common.hash.Hashing;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.Artifact;
 import java.math.BigInteger;
+import java.nio.ByteBuffer;
 
 /** Utilities for dealing with {@link ActionSketch} sketches. */
 public class Sketches {
@@ -35,16 +38,65 @@ public static BigInteger fromHashCode(HashCode hashCode) {
   public static BigInteger computeActionKey(
       ActionAnalysisMetadata action, ActionKeyContext keyContext) throws InterruptedException {
     Hasher hasher =
-        newHasher().putUnencodedChars(action.getKey(keyContext, /*artifactExpander=*/ null));
+        newHashOnlyHasher()
+            .putUnencodedChars(action.getKey(keyContext, /*artifactExpander=*/ null));
     for (Artifact output : action.getOutputs()) {
       hasher.putUnencodedChars(output.getExecPath().getPathString());
     }
     return fromHashCode(hasher.hash());
   }
 
-  public static Hasher newHasher() {
+  private static Hasher newHashOnlyHasher() {
     return Hashing.murmur3_128().newHasher();
   }
 
+  public static HashAndVersionTracker newHasher() {
+    return new HashAndVersionTrackerImpl(newHashOnlyHasher());
+  }
+
+  /** Simple interface for accumulating elements for a hash+version pair. */
+  public interface HashAndVersionTracker {
+    HashAndVersionTracker putUnencodedChars(CharSequence chars);
+
+    HashAndVersionTracker putVersion(long l);
+
+    HashAndVersionTracker putBytes(ByteBuffer bytes);
+
+    HashAndVersion hashAndVersion();
+  }
+
+  private static class HashAndVersionTrackerImpl implements HashAndVersionTracker {
+    private final Hasher hasher;
+    private long version = 0;
+
+    private HashAndVersionTrackerImpl(Hasher hasher) {
+      this.hasher = hasher;
+    }
+
+    @Override
+    public HashAndVersion hashAndVersion() {
+      return HashAndVersion.create(new BigInteger(1, hasher.hash().asBytes()), version);
+    }
+
+    @Override
+    public HashAndVersionTracker putUnencodedChars(CharSequence charSequence) {
+      hasher.putUnencodedChars(charSequence);
+      return this;
+    }
+
+    @Override
+    public HashAndVersionTracker putVersion(long v) {
+      hasher.putLong(v);
+      version = max(version, v);
+      return this;
+    }
+
+    @Override
+    public HashAndVersionTracker putBytes(ByteBuffer bytes) {
+      hasher.putBytes(bytes);
+      return this;
+    }
+  }
+
   private Sketches() {}
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionSketchFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionSketchFunction.java
@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actionsketch.ActionSketch;
+import com.google.devtools.build.lib.actionsketch.HashAndVersion;
 import com.google.devtools.build.lib.actionsketch.Sketches;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -135,14 +136,18 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
       transitiveActionKeyHash =
           BigIntegerFingerprintUtils.compose(
               transitiveActionKeyHash, depSketch.transitiveActionLookupHash());
-      transitiveSourceHash =
-          BigIntegerFingerprintUtils.composeNullable(
-              transitiveSourceHash, depSketch.transitiveSourceHash());
+      HashAndVersion hashAndVersion = depSketch.transitiveSourceHash();
+      if (hashAndVersion != null) {
+        transitiveSourceHash =
+            BigIntegerFingerprintUtils.composeNullable(transitiveSourceHash, hashAndVersion.hash());
+      } else {
+        transitiveSourceHash = null;
+      }
     }
 
     return ActionSketch.builder()
         .setTransitiveActionLookupHash(transitiveActionKeyHash)
-        .setTransitiveSourceHash(transitiveSourceHash)
+        .setTransitiveSourceHash(HashAndVersion.createNoVersion(transitiveSourceHash))
         .build();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -505,6 +505,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
         "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/actionsketch:action_sketch",
+        "//src/main/java/com/google/devtools/build/lib/actionsketch:hash_and_version",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/util",

diff --git a/src/test/java/com/google/devtools/build/lib/actionsketch/ActionSketchTest.java b/src/test/java/com/google/devtools/build/lib/actionsketch/ActionSketchTest.java
@@ -29,7 +29,7 @@ public final class ActionSketchTest {
   public void serialization() {
     roundTrip(
         ActionSketch.builder()
-            .setTransitiveSourceHash(BigInteger.ONE)
+            .setTransitiveSourceHash(HashAndVersion.create(BigInteger.ONE, /*version=*/ 0L))
             .setTransitiveActionLookupHash(new BigInteger("123456789"))
             .build());
   }
@@ -42,12 +42,12 @@ private static void roundTrip(ActionSketch sketch) {
   public void canonicalNullInstance() {
     ActionSketch sketch1 =
         ActionSketch.builder()
-            .setTransitiveSourceHash(null)
+            .setTransitiveSourceHash(HashAndVersion.create(null, /*version=*/ Long.MAX_VALUE))
             .setTransitiveActionLookupHash(null)
             .build();
     ActionSketch sketch2 =
         ActionSketch.builder()
-            .setTransitiveSourceHash(null)
+            .setTransitiveSourceHash(HashAndVersion.create(null, /*version=*/ Long.MAX_VALUE))
             .setTransitiveActionLookupHash(null)
             .build();