From 21d1bee9d9c4bccffbdbe7fd3e0c16a0ea3696ff Mon Sep 17 00:00:00 2001 From: janakr Date: Thu, 29 Apr 2021 09:33:33 -0700 Subject: [PATCH] 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 --- .../build/lib/actionsketch/ActionSketch.java | 37 +++++++++--- .../devtools/build/lib/actionsketch/BUILD | 17 +++++- .../lib/actionsketch/HashAndVersion.java | 56 +++++++++++++++++++ .../build/lib/actionsketch/Sketches.java | 56 ++++++++++++++++++- .../lib/skyframe/ActionSketchFunction.java | 13 +++-- .../google/devtools/build/lib/skyframe/BUILD | 1 + .../lib/actionsketch/ActionSketchTest.java | 6 +- 7 files changed, 168 insertions(+), 18 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/actionsketch/HashAndVersion.java 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 index 200dd52f778328..8e6cf49cd70f36 100644 --- 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 index 1fcdcf066ffbbc..9b9d6546e724ca 100644 --- 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 new file mode 100644 index 00000000000000..9f735f3963470b --- /dev/null +++ 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 index 45d43916eeac01..7a658f3bd9c218 100644 --- 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,6 +14,8 @@ 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; @@ -21,6 +23,7 @@ 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 index 66953aeded2897..71aa5b70bcd090 100644 --- 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 index 0ad6577eaec0ae..3af23b985c4495 100644 --- 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 index 253d98f740cbc7..33bc4edb558673 100644 --- 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();