Skip to content

Commit

Permalink
Simplify the decision on whether to extract include remotely. Instead…
Browse files Browse the repository at this point in the history
… of asking

the output service whether a given file is a remote file, just always extract
output files remotely.

RELNOTES: None.
PiperOrigin-RevId: 351532992
  • Loading branch information
djasper authored and copybara-github committed Jan 13, 2021
1 parent 28fa193 commit 7d7ceae
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,6 @@ 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 @@ -52,7 +52,6 @@
import com.google.devtools.build.lib.includescanning.IncludeParser.Inclusion;
import com.google.devtools.build.lib.util.io.FileOutErr;
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 java.io.IOException;
Expand All @@ -72,7 +71,6 @@ public class SpawnIncludeScanner {
ResourceSet.createWithRamCpu(/*memoryMb=*/ 10, /*cpuUsage=*/ 1);

private final Path execRoot;
private OutputService outputService;
private boolean inMemoryOutput;
private final int remoteExtractionThreshold;

Expand All @@ -82,11 +80,6 @@ public SpawnIncludeScanner(Path execRoot, int remoteExtractionThreshold) {
this.remoteExtractionThreshold = remoteExtractionThreshold;
}

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,11 +113,11 @@ public boolean shouldParseRemotely(Artifact file, ActionExecutionContext ctx) th
if (file.getRoot().getRoot().isAbsolute()) {
return false;
}
// Files written remotely that are not locally available should be scanned remotely to avoid the
// Output files are generally 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 explicitly directed to via a flag.
// remotely when the file size exceeds a certain size.
return remoteExtractionThreshold == 0
|| (outputService != null && outputService.isRemoteFile(file))
|| !file.isSourceArtifact()
|| ctx.getPathResolver().toPath(file).getFileSize() > remoteExtractionThreshold;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
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 @@ -114,13 +113,6 @@ 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,9 +141,6 @@ 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 7d7ceae

Please sign in to comment.