Skip to content

Commit

Permalink
Use PerBuildSyscallCache when getting the file size to decide on remo…
Browse files Browse the repository at this point in the history
…te include

extraction. Especially in clean builds, we'll already have stat'ed all these
files in FileStateFunction.

Also change PerBuildSyscallCache to share results independent of whether
following symlinks (FOLLOW) or not following them (NOFOLLOW). The vast majority
of files are not symlinks and so both functions return the same result.

RELNOTES: None.
PiperOrigin-RevId: 353627146
  • Loading branch information
djasper-gh authored and copybara-github committed Jan 25, 2021
1 parent 0898a2a commit 60d6f78
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ public IncludeScannerLifecycleManager(
spawnScannerSupplier.set(
new SpawnIncludeScanner(
env.getExecRoot(),
options.experimentalRemoteExtractionThreshold));
options.experimentalRemoteExtractionThreshold,
env.getSkyframeExecutor().getSyscalls()));
this.spawnScannerSupplier = spawnScannerSupplier;
env.getEventBus().register(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,18 @@
import com.google.devtools.build.lib.includescanning.IncludeParser.GrepIncludesFileType;
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.FileStatus;
import com.google.devtools.build.lib.vfs.IORuntimeException;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
import com.google.devtools.build.lib.vfs.UnixGlob.FilesystemCalls;
import java.io.IOException;
import java.io.InputStream;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicReference;
import javax.annotation.Nullable;

/**
Expand All @@ -73,11 +77,14 @@ public class SpawnIncludeScanner {
private final Path execRoot;
private boolean inMemoryOutput;
private final int remoteExtractionThreshold;
private final AtomicReference<FilesystemCalls> syscallCache;

/** Constructs a new SpawnIncludeScanner. */
public SpawnIncludeScanner(Path execRoot, int remoteExtractionThreshold) {
public SpawnIncludeScanner(
Path execRoot, int remoteExtractionThreshold, AtomicReference<FilesystemCalls> syscallCache) {
this.execRoot = execRoot;
this.remoteExtractionThreshold = remoteExtractionThreshold;
this.syscallCache = syscallCache;
}

public void setInMemoryOutput(boolean inMemoryOutput) {
Expand Down Expand Up @@ -116,9 +123,11 @@ public boolean shouldParseRemotely(Artifact file, ActionExecutionContext ctx) th
// 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 the file size exceeds a certain size.
return remoteExtractionThreshold == 0
|| !file.isSourceArtifact()
|| ctx.getPathResolver().toPath(file).getFileSize() > remoteExtractionThreshold;
if (remoteExtractionThreshold == 0 || !file.isSourceArtifact()) {
return true;
}
FileStatus status = syscallCache.get().statIfFound(file.getPath(), Symlinks.FOLLOW);
return status == null || status.getSize() > remoteExtractionThreshold;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,20 @@ public Collection<Dirent> readdir(Path path) throws IOException {

@Override
public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException {
Object result = statCache.getUnchecked(Pair.of(path, symlinks));
// Try to load a Symlinks.NOFOLLOW result first. Symlinks are rare and this enables sharing the
// cache for all non-symlink paths.
Object result = statCache.getUnchecked(Pair.of(path, Symlinks.NOFOLLOW));
if (result instanceof IOException) {
throw (IOException) result;
}
FileStatus status = (FileStatus) result;
if (status != NO_STATUS && symlinks == Symlinks.FOLLOW && status.isSymbolicLink()) {
result = statCache.getUnchecked(Pair.of(path, Symlinks.FOLLOW));
if (result instanceof IOException) {
throw (IOException) result;
}
status = (FileStatus) result;
}
return (status == NO_STATUS) ? null : status;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,10 @@ protected final PerBuildSyscallCache getPerBuildSyscallCache(int concurrencyLeve
return perBuildSyscallCache;
}

public AtomicReference<UnixGlob.FilesystemCalls> getSyscalls() {
return syscalls;
}

@ThreadCompatible
public void setActive(boolean active) {
this.active = active;
Expand Down

0 comments on commit 60d6f78

Please sign in to comment.