Skip to content

Commit

Permalink
Automated rollback of commit 7d7ceae.
Browse files Browse the repository at this point in the history
*** 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
  • Loading branch information
lberki authored and copybara-github committed Jan 26, 2021
1 parent 0a23a5e commit dafcebf
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ public void executorCreated() {
spawnScannerSupplier,
env.getExecRoot());

spawnScannerSupplier.get().setOutputService(env.getOutputService());
spawnScannerSupplier.get().setInMemoryOutput(options.inMemoryIncludesFiles);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<FilesystemCalls> syscallCache;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ void createSymlinkTree(Map<PathFragment, PathFragment> 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;
}
Expand Down

0 comments on commit dafcebf

Please sign in to comment.