From dafcebf51757de32ffaaf54c89fc3eb40561757a Mon Sep 17 00:00:00 2001 From: lberki Date: Tue, 26 Jan 2021 00:17:56 -0800 Subject: [PATCH] Automated rollback of commit 7d7ceaec50c84480617e22707304c975d8dc2940. *** Reason for rollback *** Makes Exoblaze slower: https://storage.cloud.google.com/blaze-performance-data/2021-01-19/nightly_report.html#yt-ios-local-blaze-bench-imacpro-2017-18c *** Original change description *** Simplify the decision on whether to extract include remotely. Instead of asking the output service whether a given file is a remote file, just always extract output files remotely. RELNOTES: None. PiperOrigin-RevId: 353812770 --- .../lib/includescanning/IncludeScanningModule.java | 1 + .../lib/includescanning/SpawnIncludeScanner.java | 14 +++++++++++--- .../build/lib/remote/RemoteOutputService.java | 8 ++++++++ .../devtools/build/lib/vfs/OutputService.java | 3 +++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java index 97a30124fe89a4..5d15e28b8e6a5d 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScanningModule.java @@ -304,6 +304,7 @@ public void executorCreated() { spawnScannerSupplier, env.getExecRoot()); + spawnScannerSupplier.get().setOutputService(env.getOutputService()); spawnScannerSupplier.get().setInMemoryOutput(options.inMemoryIncludesFiles); } } diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java index bcde047143966f..74c0c9125728f4 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java @@ -53,6 +53,7 @@ import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.IORuntimeException; +import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; @@ -75,6 +76,7 @@ public class SpawnIncludeScanner { ResourceSet.createWithRamCpu(/*memoryMb=*/ 10, /*cpuUsage=*/ 1); private final Path execRoot; + private OutputService outputService; private boolean inMemoryOutput; private final int remoteExtractionThreshold; private final AtomicReference syscallCache; @@ -87,6 +89,11 @@ public SpawnIncludeScanner( this.syscallCache = syscallCache; } + public void setOutputService(OutputService outputService) { + Preconditions.checkState(this.outputService == null); + this.outputService = outputService; + } + public void setInMemoryOutput(boolean inMemoryOutput) { this.inMemoryOutput = inMemoryOutput; } @@ -120,10 +127,11 @@ public boolean shouldParseRemotely(Artifact file, ActionExecutionContext ctx) th if (file.getRoot().getRoot().isAbsolute()) { return false; } - // Output files are generally not locally available should be scanned remotely to avoid the + // Files written remotely that are not locally available should be scanned remotely to avoid the // bandwidth and disk space penalty of bringing them across. Also, enable include scanning - // remotely when the file size exceeds a certain size. - if (remoteExtractionThreshold == 0 || !file.isSourceArtifact()) { + // remotely when explicitly directed to via a flag. + if (remoteExtractionThreshold == 0 + || (outputService != null && outputService.isRemoteFile(file))) { return true; } FileStatus status = syscallCache.get().statIfFound(file.getPath(), Symlinks.FOLLOW); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index 6939e1ea821015..957af4e4e45089 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.OutputService; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import java.util.Map; @@ -113,6 +114,13 @@ public void clean() { // Intentionally left empty. } + @Override + public boolean isRemoteFile(Artifact artifact) { + Path path = artifact.getPath(); + return path.getFileSystem() instanceof RemoteActionFileSystem + && ((RemoteActionFileSystem) path.getFileSystem()).isRemote(path); + } + @Override public boolean supportsPathResolverForArtifactValues() { return actionFileSystemType() != ActionFileSystemType.DISABLED; diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index 21da650303de0b..88a4cfffcf226c 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -141,6 +141,9 @@ void createSymlinkTree(Map symlinks, PathFragment sy */ void clean() throws ExecException, InterruptedException; + /** @return true iff the file actually lives on a remote server */ + boolean isRemoteFile(Artifact file); + default ActionFileSystemType actionFileSystemType() { return ActionFileSystemType.DISABLED; }