Make Digest (renamed Md5Digest) a little more multi-purpose. -- MOS_MIGRATED_REVID=130986194
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index b96a9e4..5b913ad 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -19,7 +19,7 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType; import com.google.devtools.build.lib.actions.cache.ActionCache; import com.google.devtools.build.lib.actions.cache.ActionCache.Entry; -import com.google.devtools.build.lib.actions.cache.Digest; +import com.google.devtools.build.lib.actions.cache.DigestUtils; import com.google.devtools.build.lib.actions.cache.Metadata; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.events.Event; @@ -108,7 +108,7 @@ for (Artifact artifact : artifacts) { mdMap.put(artifact.getExecPathString(), metadataHandler.getMetadataMaybe(artifact)); } - return !Digest.fromMetadata(mdMap).equals(entry.getFileDigest()); + return !DigestUtils.fromMetadata(mdMap).equals(entry.getFileDigest()); } private void reportCommand(EventHandler handler, Action action) {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java index b87dd84..5dc88cf 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
@@ -19,7 +19,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; - import java.io.IOException; import java.io.PrintStream; import java.util.ArrayList; @@ -28,7 +27,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; - import javax.annotation.Nullable; /** @@ -73,9 +71,9 @@ @Nullable // Null iff the corresponding action does not do input discovery. private final List<String> files; - // If null, digest is non-null and the entry is immutable. + // If null, md5Digest is non-null and the entry is immutable. private Map<String, Metadata> mdMap; - private Digest digest; + private Md5Digest md5Digest; public Entry(String key, boolean discoversInputs) { actionKey = key; @@ -83,10 +81,10 @@ mdMap = new HashMap<>(); } - public Entry(String key, @Nullable List<String> files, Digest digest) { + public Entry(String key, @Nullable List<String> files, Md5Digest md5Digest) { actionKey = key; this.files = files; - this.digest = digest; + this.md5Digest = md5Digest; mdMap = null; } @@ -97,7 +95,7 @@ public void addFile(PathFragment relativePath, Metadata md) { Preconditions.checkState(mdMap != null); Preconditions.checkState(!isCorrupted()); - Preconditions.checkState(digest == null); + Preconditions.checkState(md5Digest == null); String execPath = relativePath.getPathString(); if (discoversInputs()) { @@ -114,17 +112,17 @@ } /** - * Returns the combined digest of the action's inputs and outputs. + * Returns the combined md5Digest of the action's inputs and outputs. * - * This may compresses the data into a more compact representation, and - * makes the object immutable. + * <p>This may compresses the data into a more compact representation, and makes the object + * immutable. */ - public Digest getFileDigest() { - if (digest == null) { - digest = Digest.fromMetadata(mdMap); + public Md5Digest getFileDigest() { + if (md5Digest == null) { + md5Digest = DigestUtils.fromMetadata(mdMap); mdMap = null; } - return digest; + return md5Digest; } /** @@ -153,10 +151,10 @@ StringBuilder builder = new StringBuilder(); builder.append(" actionKey = ").append(actionKey).append("\n"); builder.append(" digestKey = "); - if (digest == null) { - builder.append(Digest.fromMetadata(mdMap)).append(" (from mdMap)\n"); + if (md5Digest == null) { + builder.append(DigestUtils.fromMetadata(mdMap)).append(" (from mdMap)\n"); } else { - builder.append(digest).append("\n"); + builder.append(md5Digest).append("\n"); } if (discoversInputs()) {
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index 8cbfd77..f0b47f8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.util.VarInt; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.UnixGlob; - import java.io.ByteArrayOutputStream; import java.io.DataInputStream; import java.io.DataOutputStream; @@ -352,14 +351,18 @@ // + 16 bytes for the digest // + 5 bytes max for the file list length // + 5 bytes max for each file id - int maxSize = VarInt.MAX_VARINT_SIZE + actionKeyBytes.length + Digest.MD5_SIZE - + VarInt.MAX_VARINT_SIZE + files.size() * VarInt.MAX_VARINT_SIZE; + int maxSize = + VarInt.MAX_VARINT_SIZE + + actionKeyBytes.length + + Md5Digest.MD5_SIZE + + VarInt.MAX_VARINT_SIZE + + files.size() * VarInt.MAX_VARINT_SIZE; ByteArrayOutputStream sink = new ByteArrayOutputStream(maxSize); VarInt.putVarInt(actionKeyBytes.length, sink); sink.write(actionKeyBytes); - entry.getFileDigest().write(sink); + DigestUtils.write(entry.getFileDigest(), sink); VarInt.putVarInt(entry.discoversInputs() ? files.size() : NO_INPUT_DISCOVERY_COUNT, sink); for (String file : files) { @@ -385,7 +388,7 @@ source.get(actionKeyBytes); String actionKey = new String(actionKeyBytes, ISO_8859_1); - Digest digest = Digest.read(source); + Md5Digest md5Digest = DigestUtils.read(source); int count = VarInt.getVarInt(source); ImmutableList.Builder<String> builder = new ImmutableList.Builder<>(); @@ -400,8 +403,8 @@ if (source.remaining() > 0) { throw new IOException("serialized entry data has not been fully decoded"); } - return new Entry(actionKey, - count == NO_INPUT_DISCOVERY_COUNT ? null : builder.build(), digest); + return new Entry( + actionKey, count == NO_INPUT_DISCOVERY_COUNT ? null : builder.build(), md5Digest); } catch (BufferUnderflowException e) { throw new IOException("encoded entry data is incomplete", e); }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/Digest.java b/src/main/java/com/google/devtools/build/lib/actions/cache/Digest.java deleted file mode 100644 index 0698919..0000000 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/Digest.java +++ /dev/null
@@ -1,138 +0,0 @@ -// Copyright 2014 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.actions.cache; - -import com.google.common.hash.HashCode; -import com.google.devtools.build.lib.util.Fingerprint; -import com.google.devtools.build.lib.util.VarInt; - -import java.io.IOException; -import java.io.OutputStream; -import java.nio.ByteBuffer; -import java.util.Arrays; -import java.util.Map; - -/** - * A value class for capturing and comparing MD5-based digests. - * - * <p>Note that this class is responsible for digesting file metadata in an - * order-independent manner. Care must be taken to do this properly. The - * digest must be a function of the set of (path, metadata) tuples. While the - * order of these pairs must not matter, it would <b>not</b> be safe to make - * the digest be a function of the set of paths and the set of metadata. - * - * <p>Note that the (path, metadata) tuples must be unique, otherwise the - * XOR-based approach will fail. - */ -public class Digest { - - static final int MD5_SIZE = 16; - - private final byte[] digest; - - /** - * Construct the digest from the given bytes. - * @param digest an MD5 digest. Must be sized properly. - */ - private Digest(byte[] digest) { - this.digest = digest; - } - - /** - * @param source the byte buffer source. - * @return the digest from the given buffer. - * @throws IOException if the byte buffer is incorrectly formatted. - */ - public static Digest read(ByteBuffer source) throws IOException { - int size = VarInt.getVarInt(source); - if (size != MD5_SIZE) { - throw new IOException("Unexpected digest length: " + size); - } - byte[] bytes = new byte[size]; - source.get(bytes); - return new Digest(bytes); - } - - /** - * Write the digest to the output stream. - */ - public void write(OutputStream sink) throws IOException { - VarInt.putVarInt(digest.length, sink); - sink.write(digest); - } - - /** - * @param mdMap A collection of (execPath, Metadata) pairs. - * Values may be null. - * @return an <b>order-independent</b> digest from the given "set" of - * (path, metadata) pairs. - */ - public static Digest fromMetadata(Map<String, Metadata> mdMap) { - byte[] result = new byte[MD5_SIZE]; - // Profiling showed that MD5 engine instantiation was a hotspot, so create one instance for - // this computation to amortize its cost. - Fingerprint fp = new Fingerprint(); - for (Map.Entry<String, Metadata> entry : mdMap.entrySet()) { - xorWith(result, getDigest(fp, entry.getKey(), entry.getValue())); - } - return new Digest(result); - } - - /** - * @return this Digest as a Metadata with no mtime. - */ - public Metadata asMetadata() { - return new Metadata(digest); - } - - @Override - public int hashCode() { - return Arrays.hashCode(digest); - } - - @Override - public boolean equals(Object obj) { - return (obj instanceof Digest) && Arrays.equals(digest, ((Digest) obj).digest); - } - - @Override - public String toString() { - return HashCode.fromBytes(digest).toString(); - } - - private static byte[] getDigest(Fingerprint fp, String execPath, Metadata md) { - fp.addStringLatin1(execPath); - - if (md == null) { - // Move along, nothing to see here. - } else if (md.digest == null) { - // Use the timestamp if the digest is not present, but not both. - // Modifying a timestamp while keeping the contents of a file the - // same should not cause rebuilds. - fp.addLong(md.mtime); - } else { - fp.addBytes(md.digest); - } - return fp.digestAndReset(); - } - - /** - * Compute lhs ^= rhs bitwise operation of the arrays. - */ - private static void xorWith(byte[] lhs, byte[] rhs) { - for (int i = 0; i < lhs.length; i++) { - lhs[i] ^= rhs[i]; - } - } -}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java b/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java index c61e943..ef4d9dc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/DigestUtils.java
@@ -17,21 +17,33 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.util.BlazeClock; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.LoggingUtil; import com.google.devtools.build.lib.util.Preconditions; +import com.google.devtools.build.lib.util.VarInt; import com.google.devtools.build.lib.vfs.Path; - import java.io.IOException; +import java.io.OutputStream; +import java.nio.ByteBuffer; +import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; - import javax.annotation.Nullable; /** * Utility class for getting md5 digests of files. + * + * <p>Note that this class is responsible for digesting file metadata in an order-independent + * manner. Care must be taken to do this properly. The digest must be a function of the set of + * (path, metadata) tuples. While the order of these pairs must not matter, it would <b>not</b> be + * safe to make the digest be a function of the set of paths and the set of metadata. + * + * <p>Note that the (path, metadata) tuples must be unique, otherwise the XOR-based approach will + * fail. */ public class DigestUtils { + // Object to synchronize on when serializing large file reads. private static final Object MD5_LOCK = new Object(); private static final AtomicBoolean MULTI_THREADED_DIGEST = new AtomicBoolean(false); @@ -122,4 +134,63 @@ return getDigestInternal(path); } } + + /** + * @param source the byte buffer source. + * @return the digest from the given buffer. + * @throws IOException if the byte buffer is incorrectly formatted. + */ + public static Md5Digest read(ByteBuffer source) throws IOException { + int size = VarInt.getVarInt(source); + if (size != Md5Digest.MD5_SIZE) { + throw new IOException("Unexpected digest length: " + size); + } + byte[] bytes = new byte[size]; + source.get(bytes); + return new Md5Digest(bytes); + } + + /** Write the digest to the output stream. */ + public static void write(Md5Digest digest, OutputStream sink) throws IOException { + VarInt.putVarInt(digest.getDigestBytesUnsafe().length, sink); + sink.write(digest.getDigestBytesUnsafe()); + } + + /** + * @param mdMap A collection of (execPath, Metadata) pairs. Values may be null. + * @return an <b>order-independent</b> digest from the given "set" of (path, metadata) pairs. + */ + public static Md5Digest fromMetadata(Map<String, Metadata> mdMap) { + byte[] result = new byte[Md5Digest.MD5_SIZE]; + // Profiling showed that MD5 engine instantiation was a hotspot, so create one instance for + // this computation to amortize its cost. + Fingerprint fp = new Fingerprint(); + for (Map.Entry<String, Metadata> entry : mdMap.entrySet()) { + xorWith(result, getDigest(fp, entry.getKey(), entry.getValue())); + } + return new Md5Digest(result); + } + + private static byte[] getDigest(Fingerprint fp, String execPath, Metadata md) { + fp.addStringLatin1(execPath); + + if (md == null) { + // Move along, nothing to see here. + } else if (md.digest == null) { + // Use the timestamp if the digest is not present, but not both. + // Modifying a timestamp while keeping the contents of a file the + // same should not cause rebuilds. + fp.addLong(md.mtime); + } else { + fp.addBytes(md.digest); + } + return fp.digestAndReset(); + } + + /** Compute lhs ^= rhs bitwise operation of the arrays. */ + private static void xorWith(byte[] lhs, byte[] rhs) { + for (int i = 0; i < lhs.length; i++) { + lhs[i] ^= rhs[i]; + } + } }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/Md5Digest.java b/src/main/java/com/google/devtools/build/lib/actions/cache/Md5Digest.java new file mode 100644 index 0000000..75441b5 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/Md5Digest.java
@@ -0,0 +1,59 @@ +// Copyright 2014 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.actions.cache; + +import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.util.Preconditions; +import java.util.Arrays; + +/** A value class for capturing and comparing MD5-based digests. */ +public class Md5Digest { + + static final int MD5_SIZE = 16; + private final byte[] digest; + + /** + * Construct the digest from the given bytes. + * + * @param digest an MD5 digest. Must be sized properly. + */ + public Md5Digest(byte[] digest) { + Preconditions.checkState(digest.length == MD5_SIZE); + this.digest = digest; + } + + @Override + public int hashCode() { + // We are already dealing with the digest so we can just use portion of it as a hash code. + return digest[0] + (digest[1] << 8) + (digest[2] << 16) + (digest[3] << 24); + } + + @Override + public boolean equals(Object obj) { + return (obj instanceof Md5Digest) && Arrays.equals(digest, ((Md5Digest) obj).digest); + } + + @Override + public String toString() { + return HashCode.fromBytes(digest).toString(); + } + + /** + * Warning: This is a mutable wrapper and does not necessarily own the underlying byte[]. Don't + * modify it unless you are sure about it. + */ + public byte[] getDigestBytesUnsafe() { + return digest; + } +}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java b/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java index 2189130..316fd23 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/Metadata.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.util.Preconditions; - import java.util.Arrays; import java.util.Date;
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java index 6fcb964..5e74180 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/MetadataHandler.java
@@ -18,7 +18,6 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.MiddlemanAction; import com.google.devtools.build.lib.vfs.FileStatus; - import java.io.IOException; /** Retrieves {@link Metadata} of {@link Artifact}s, and inserts virtual metadata as well. */ @@ -46,8 +45,8 @@ */ Metadata getMetadata(Artifact artifact) throws IOException; - /** Sets digest for virtual artifacts (e.g. middlemen). {@code digest} must not be null. */ - void setDigestForVirtualArtifact(Artifact artifact, Digest digest); + /** Sets digest for virtual artifacts (e.g. middlemen). {@code md5Digest} must not be null. */ + void setDigestForVirtualArtifact(Artifact artifact, Md5Digest md5Digest); /** * Registers the given output as contents of a TreeArtifact, without injecting its digest.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index dd93e04..087e97a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -23,7 +23,7 @@ import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; -import com.google.devtools.build.lib.actions.cache.Digest; +import com.google.devtools.build.lib.actions.cache.Md5Digest; import com.google.devtools.build.lib.actions.cache.Metadata; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.skyframe.TreeArtifactValue.TreeArtifactException; @@ -264,11 +264,11 @@ } @Override - public void setDigestForVirtualArtifact(Artifact artifact, Digest digest) { + public void setDigestForVirtualArtifact(Artifact artifact, Md5Digest md5Digest) { Preconditions.checkArgument(artifact.isMiddlemanArtifact(), artifact); - Preconditions.checkNotNull(digest, artifact); - additionalOutputData.put(artifact, - FileArtifactValue.createProxy(digest.asMetadata().digest)); + Preconditions.checkNotNull(md5Digest, artifact); + additionalOutputData.put( + artifact, FileArtifactValue.createProxy(md5Digest.getDigestBytesUnsafe())); } private Set<TreeFileArtifact> getTreeArtifactContents(Artifact artifact) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java index e4ee468..b4fe3ee 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TreeArtifactValue.java
@@ -22,7 +22,7 @@ import com.google.common.collect.Maps; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; -import com.google.devtools.build.lib.actions.cache.Digest; +import com.google.devtools.build.lib.actions.cache.DigestUtils; import com.google.devtools.build.lib.actions.cache.Metadata; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -69,7 +69,7 @@ } return new TreeArtifactValue( - Digest.fromMetadata(digestBuilder).asMetadata().digest, + DigestUtils.fromMetadata(digestBuilder).getDigestBytesUnsafe(), ImmutableMap.copyOf(childFileValues)); }