From 01776ee8495043816e6224ebeff2756d34db41cb Mon Sep 17 00:00:00 2001 From: ulfjack Date: Mon, 26 Jun 2017 10:33:05 +0200 Subject: Make Metadata an interface for FileArtifactValue Replace all previous uses of Metadata with FileArtifactValue (or a simple inner class in the case of ActionCacheChecker.CONSTANT_METADATA). Care was taken to make the equals method obey the equals contract, even in the presence of multiple implementations. PiperOrigin-RevId: 160115080 --- .../build/lib/skyframe/FileArtifactValue.java | 158 +++++++++++---------- 1 file changed, 84 insertions(+), 74 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java') diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java index cfdcb187de..2f06471ce4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileArtifactValue.java @@ -17,6 +17,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.cache.DigestUtils; +import com.google.devtools.build.lib.actions.cache.Metadata; +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 com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.Path; @@ -39,8 +42,9 @@ import javax.annotation.Nullable; * */ // TODO(janakr): make this an interface once JDK8 allows us to have static methods on interfaces. -public abstract class FileArtifactValue implements SkyValue { - private static final class SingletonMarkerValue extends FileArtifactValue { +@Immutable @ThreadSafe +public abstract class FileArtifactValue implements SkyValue, Metadata { + private static final class SingletonMarkerValue extends FileArtifactValue implements Singleton { @Nullable @Override public byte[] getDigest() { @@ -48,7 +52,7 @@ public abstract class FileArtifactValue implements SkyValue { } @Override - boolean isFile() { + public boolean isFile() { return false; } @@ -68,6 +72,33 @@ public abstract class FileArtifactValue implements SkyValue { } } + private static final class OmittedFileValue extends FileArtifactValue implements Singleton { + @Override + public byte[] getDigest() { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isFile() { + throw new UnsupportedOperationException(); + } + + @Override + public long getSize() { + throw new UnsupportedOperationException(); + } + + @Override + public long getModifiedTime() { + throw new UnsupportedOperationException(); + } + + @Override + public String toString() { + return "OMITTED_FILE_MARKER"; + } + } + static final FileArtifactValue DEFAULT_MIDDLEMAN = new SingletonMarkerValue(); /** Data that marks that a file is not present on the filesystem. */ @VisibleForTesting @@ -77,33 +108,7 @@ public abstract class FileArtifactValue implements SkyValue { * Represents an omitted file -- we are aware of it but it doesn't exist. All access methods are * unsupported. */ - static final FileArtifactValue OMITTED_FILE_MARKER = - new FileArtifactValue() { - @Override - public byte[] getDigest() { - throw new UnsupportedOperationException(); - } - - @Override - boolean isFile() { - throw new UnsupportedOperationException(); - } - - @Override - public long getSize() { - throw new UnsupportedOperationException(); - } - - @Override - public long getModifiedTime() { - throw new UnsupportedOperationException(); - } - - @Override - public String toString() { - return "OMITTED_FILE_MARKER"; - } - }; + static final FileArtifactValue OMITTED_FILE_MARKER = new OmittedFileValue(); private static final class DirectoryArtifactValue extends FileArtifactValue { private final long mtime; @@ -133,18 +138,6 @@ public abstract class FileArtifactValue implements SkyValue { return false; } - @Override - public int hashCode() { - return (int) mtime; - } - - @Override - public boolean equals(Object other) { - return (this == other) - || ((other instanceof DirectoryArtifactValue) - && this.mtime == ((DirectoryArtifactValue) other).mtime); - } - @Override public String toString() { return MoreObjects.toStringHelper(this).add("mtime", mtime).toString(); @@ -166,7 +159,7 @@ public abstract class FileArtifactValue implements SkyValue { } @Override - boolean isFile() { + public boolean isFile() { return true; } @@ -181,29 +174,6 @@ public abstract class FileArtifactValue implements SkyValue { "regular file's mtime should never be called. (" + this + ")"); } - @Override - public int hashCode() { - // Hash digest by content, not reference. - return 37 * (int) size + Arrays.hashCode(digest); - } - - /** - * Two RegularFileArtifactValues will only compare equal if they have the same content. This - * differs from the {@code Metadata#equivalence} method, which allows for comparison using mtime - * if one object does not have a digest available. - */ - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (!(other instanceof RegularFileArtifactValue)) { - return false; - } - RegularFileArtifactValue that = (RegularFileArtifactValue) other; - return this.size == that.size && Arrays.equals(this.digest, that.digest); - } - @Override public String toString() { return MoreObjects.toStringHelper(this).add("digest", digest).add("size", size).toString(); @@ -239,10 +209,18 @@ public abstract class FileArtifactValue implements SkyValue { return createNormalFile(digest, size); } - static FileArtifactValue createNormalFile(byte[] digest, long size) { + public static FileArtifactValue createNormalFile(byte[] digest, long size) { return new RegularFileArtifactValue(digest, size); } + static FileArtifactValue createNormalFile(FileValue fileValue) { + return new RegularFileArtifactValue(fileValue.getDigest(), fileValue.getSize()); + } + + public static FileArtifactValue createDirectory(long mtime) { + return new DirectoryArtifactValue(mtime); + } + /** * Creates a FileArtifactValue used as a 'proxy' input for other ArtifactValues. * These are used in {@link com.google.devtools.build.lib.actions.ActionCacheChecker}. @@ -252,16 +230,48 @@ public abstract class FileArtifactValue implements SkyValue { return createNormalFile(digest, /*size=*/ 0); } - /** Returns the digest of this value. Null for non-files, non-null for files. */ + @Override + public abstract boolean isFile(); + @Nullable + @Override public abstract byte[] getDigest(); - /** @return true if this is a file or a symlink to an existing file */ - abstract boolean isFile(); - - /** Gets the size of the file. Non-files (including directories) have size 0. */ + @Override public abstract long getSize(); - /** Gets last modified time of file. Should only be called if this is not a file. */ - abstract long getModifiedTime(); + @Override + public abstract long getModifiedTime(); + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof Metadata)) { + return false; + } + if ((this instanceof Singleton) || (o instanceof Singleton)) { + return false; + } + Metadata m = (Metadata) o; + if (isFile()) { + return m.isFile() && Arrays.equals(getDigest(), m.getDigest()) && getSize() == m.getSize(); + } else { + return !m.isFile() && getModifiedTime() == m.getModifiedTime(); + } + } + + @Override + public int hashCode() { + if (this instanceof Singleton) { + return System.identityHashCode(this); + } + // Hash digest by content, not reference. + if (isFile()) { + return 37 * Long.hashCode(getSize()) + Arrays.hashCode(getDigest()); + } else { + return Long.hashCode(getModifiedTime()); + } + } } -- cgit v1.2.3