From f1af869e25dba1d189b1c5dc8b63aeb88f14eda8 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 5 May 2023 02:52:16 -0700 Subject: [PATCH] Rename RemoteFileStatus to FileStatusWithMetadata and make it able to return any file metadata rather than remote. Also make RemoteFileInfo store the metadata directly so we don't need to convert back and forth. PiperOrigin-RevId: 529665424 Change-Id: I038ef07b6b328f3fc641e73d2d7b1f5ab3c255b6 --- .../google/devtools/build/lib/actions/BUILD | 4 +- ...tatus.java => FileStatusWithMetadata.java} | 13 +++-- .../lib/remote/RemoteActionFileSystem.java | 53 +++++++++---------- .../lib/skyframe/ActionMetadataHandler.java | 6 +-- 4 files changed, 37 insertions(+), 39 deletions(-) rename src/main/java/com/google/devtools/build/lib/actions/{RemoteFileStatus.java => FileStatusWithMetadata.java} (63%) diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index ce619757a54523..ff6058adb986f6 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -65,7 +65,7 @@ java_library( "ResourceManager.java", "ResourceSet.java", "ResourceSetOrBuilder.java", - "RemoteFileStatus.java", + "FileStatusWithMetadata.java", "SharedActionEvent.java", "PackageRootResolver.java", "PackageRoots.java", @@ -306,9 +306,9 @@ java_library( "FileContentsProxy.java", "FileStateType.java", "FileStateValue.java", + "FileStatusWithMetadata.java", "FileValue.java", "RemoteArtifactChecker.java", - "RemoteFileStatus.java", "cache/MetadataDigestUtils.java", ], deps = [ diff --git a/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java b/src/main/java/com/google/devtools/build/lib/actions/FileStatusWithMetadata.java similarity index 63% rename from src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java rename to src/main/java/com/google/devtools/build/lib/actions/FileStatusWithMetadata.java index 73184b55e7cc30..085b3f7ff663cc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FileStatusWithMetadata.java @@ -13,16 +13,15 @@ // limitations under the License. package com.google.devtools.build.lib.actions; -import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.vfs.FileStatusWithDigest; /** - * A FileStatus that exists remotely and provides remote metadata. + * A FileStatus that provides metadata. * - *

Filesystem may return implementation of this interface if the files are stored remotely. When - * checking outputs of actions, Skyframe will inject the metadata returned by {@link - * #getRemoteMetadata()} if the output file has {@link RemoteFileStatus}. + *

Filesystem may return implementation of this interface if the files are stored in the memory. + * When checking outputs of actions, Skyframe will inject the metadata returned by {@link + * #getMetadata()} instead of construct a new metadata from the status. */ -public interface RemoteFileStatus extends FileStatusWithDigest { - RemoteFileArtifactValue getRemoteMetadata(); +public interface FileStatusWithMetadata extends FileStatusWithDigest { + FileArtifactValue getMetadata(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 7e4096b7fb3e5d..37c9bfbf5e1f25 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -34,8 +34,8 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.actions.FileStatusWithMetadata; import com.google.devtools.build.lib.actions.InputMetadataProvider; -import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; @@ -247,14 +247,6 @@ private void maybeInjectMetadataForSymlinkOrDownload(PathFragment linkPath, Arti } } - private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) { - return RemoteFileArtifactValue.create( - remoteFile.getFastDigest(), - remoteFile.getSize(), - /* locationIndex= */ 1, - remoteFile.getExpireAtEpochMilli()); - } - @Override public String getFileSystemType(PathFragment path) { return "remoteActionFS"; @@ -520,7 +512,7 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx } private static FileStatus statFromRemoteMetadata(RemoteFileArtifactValue m) { - return new RemoteFileStatus() { + return new FileStatusWithMetadata() { @Override public byte[] getDigest() { return m.getDigest(); @@ -567,7 +559,7 @@ public long getNodeId() { } @Override - public RemoteFileArtifactValue getRemoteMetadata() { + public RemoteFileArtifactValue getMetadata() { return m; } }; @@ -596,7 +588,7 @@ public FileArtifactValue getOutputMetadataForTopLevelArtifactDownloader(ActionIn remoteOutputTree.getRemoteFileInfo( execRoot.getRelative(input.getExecPath()), /* followSymlinks= */ true); if (remoteFile != null) { - return createRemoteMetadata(remoteFile); + return remoteFile.getMetadata(); } // TODO(tjgq): This should not work. @@ -627,7 +619,7 @@ private FileArtifactValue getMetadataByExecPath(PathFragment execPath) { remoteOutputTree.getRemoteFileInfo( execRoot.getRelative(execPath), /* followSymlinks= */ true); if (remoteFile != null) { - return createRemoteMetadata(remoteFile); + return remoteFile.getMetadata(); } return null; @@ -846,7 +838,10 @@ void injectRemoteFile(PathFragment path, byte[] digest, long size, long expireAt } RemoteFileInfo remoteFileInfo = (RemoteFileInfo) node; - remoteFileInfo.set(digest, size, expireAtEpochMilli); + + var metadata = + RemoteFileArtifactValue.create(digest, size, /* locationIndex= */ 1, expireAtEpochMilli); + remoteFileInfo.set(metadata); } @Nullable @@ -859,21 +854,15 @@ RemoteFileInfo getRemoteFileInfo(PathFragment path, boolean followSymlinks) { } } - static class RemoteFileInfo extends FileInfo { - - private byte[] digest; - private long size; - - private long expireAtEpochMilli; + static class RemoteFileInfo extends FileInfo implements FileStatusWithMetadata { + private RemoteFileArtifactValue metadata; RemoteFileInfo(Clock clock) { super(clock); } - private void set(byte[] digest, long size, long expireAtEpochMilli) { - this.digest = digest; - this.size = size; - this.expireAtEpochMilli = expireAtEpochMilli; + private void set(RemoteFileArtifactValue metadata) { + this.metadata = metadata; } @Override @@ -893,16 +882,26 @@ public byte[] getxattr(String name) throws IOException { @Override public byte[] getFastDigest() { - return digest; + return metadata.getDigest(); + } + + @Override + public byte[] getDigest() throws IOException { + return metadata.getDigest(); } @Override public long getSize() { - return size; + return metadata.getSize(); } public long getExpireAtEpochMilli() { - return expireAtEpochMilli; + return metadata.getExpireAtEpochMilli(); + } + + @Override + public RemoteFileArtifactValue getMetadata() { + return metadata; } } } 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 a4ecab69b6f9d9..9b22f4d4196438 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 @@ -36,11 +36,11 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileStateType; import com.google.devtools.build.lib.actions.FileStateValue; +import com.google.devtools.build.lib.actions.FileStatusWithMetadata; import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.InputMetadataProvider; -import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.actions.cache.OutputMetadataStore; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.DigestUtils; @@ -682,8 +682,8 @@ private static FileArtifactValue fileArtifactValueFromStat( return FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime()); } - if (stat instanceof RemoteFileStatus) { - return ((RemoteFileStatus) stat).getRemoteMetadata(); + if (stat instanceof FileStatusWithMetadata) { + return ((FileStatusWithMetadata) stat).getMetadata(); } FileStateValue fileStateValue =