Skip to content

Commit

Permalink
Make references to syscall caches less confusing.
Browse files Browse the repository at this point in the history
`PerBuildSyscallCache` is renamed `DefaultSyscallCache`. It is server-scoped, not per-build scoped, although `clear()` is called between builds (this is now documented on `SyscallCache`).

Variables named `perBuild` or `perCommand` are adjusted accordingly.

PiperOrigin-RevId: 493672664
Change-Id: Ifa70d8c636fec4b9bc9e194691f865569de3ff2a
  • Loading branch information
justinhorvitz authored and copybara-github committed Dec 7, 2022
1 parent 5adbfd4 commit 8a53b0e
Show file tree
Hide file tree
Showing 27 changed files with 86 additions and 87 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,14 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:configuration_phase_started_event",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_progress_receiver",
"//src/main/java/com/google/devtools/build/lib/skyframe:default_syscall_cache",
"//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
"//src/main/java/com/google/devtools/build/lib/skyframe:execution_finished_event",
"//src/main/java/com/google/devtools/build/lib/skyframe:incremental_package_roots",
"//src/main/java/com/google/devtools/build/lib/skyframe:loading_phase_started_event",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_progress_receiver",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_roots_no_symlink_creation",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:per_build_syscall_cache",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public final class BlazeWorkspace {

private final BlazeDirectories directories;
private final SkyframeExecutor skyframeExecutor;
private final SyscallCache perCommandSyscallCache;
private final SyscallCache syscallCache;

/**
* Loaded lazily on the first build command that enables the action cache. Cleared on a build
Expand All @@ -84,7 +84,7 @@ public BlazeWorkspace(
WorkspaceStatusAction.Factory workspaceStatusActionFactory,
BinTools binTools,
@Nullable AllocationTracker allocationTracker,
SyscallCache perCommandSyscallCache) {
SyscallCache syscallCache) {
this.runtime = runtime;
this.eventBusExceptionHandler = Preconditions.checkNotNull(eventBusExceptionHandler);
this.workspaceStatusActionFactory = workspaceStatusActionFactory;
Expand All @@ -93,7 +93,7 @@ public BlazeWorkspace(

this.directories = directories;
this.skyframeExecutor = skyframeExecutor;
this.perCommandSyscallCache = perCommandSyscallCache;
this.syscallCache = syscallCache;

if (directories.inWorkspace()) {
writeOutputBaseReadmeFile();
Expand Down Expand Up @@ -211,7 +211,7 @@ public CommandEnvironment initCommand(
Thread.currentThread(),
command,
options,
perCommandSyscallCache,
syscallCache,
warnings,
waitTimeInMs,
commandStartTime,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
import com.google.devtools.build.lib.exec.BinTools;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.profiler.memory.AllocationTracker;
import com.google.devtools.build.lib.skyframe.DefaultSyscallCache;
import com.google.devtools.build.lib.skyframe.DiffAwareness;
import com.google.devtools.build.lib.skyframe.PerBuildSyscallCache;
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutorFactory;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyframeExecutorFactory;
Expand Down Expand Up @@ -60,7 +60,7 @@ public final class WorkspaceBuilder {
private SkyframeExecutorRepositoryHelpersHolder skyframeExecutorRepositoryHelpersHolder = null;

@Nullable private SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver = null;
private SyscallCache perCommandSyscallCache;
private SyscallCache syscallCache;

WorkspaceBuilder(BlazeDirectories directories, BinTools binTools) {
this.directories = directories;
Expand Down Expand Up @@ -88,12 +88,6 @@ public static int getSyscallCacheInitialCapacity() {
return (int) scaledMemory;
}

public static PerBuildSyscallCache createPerBuildSyscallCache() {
return PerBuildSyscallCache.newBuilder()
.setInitialCapacity(getSyscallCacheInitialCapacity())
.build();
}

BlazeWorkspace build(
BlazeRuntime runtime,
PackageFactory packageFactory,
Expand All @@ -103,12 +97,15 @@ BlazeWorkspace build(
if (skyframeExecutorFactory == null) {
skyframeExecutorFactory = new SequencedSkyframeExecutorFactory();
}
if (perCommandSyscallCache == null) {
perCommandSyscallCache = createPerBuildSyscallCache();
if (syscallCache == null) {
syscallCache =
DefaultSyscallCache.newBuilder()
.setInitialCapacity(getSyscallCacheInitialCapacity())
.build();
}

SingleFileSystemSyscallCache singleFsSyscallCache =
new SingleFileSystemSyscallCache(perCommandSyscallCache, runtime.getFileSystem());
new SingleFileSystemSyscallCache(syscallCache, runtime.getFileSystem());

SkyframeExecutor skyframeExecutor =
skyframeExecutorFactory.create(
Expand Down Expand Up @@ -173,13 +170,10 @@ public WorkspaceBuilder setAllocationTracker(AllocationTracker allocationTracker
}

@CanIgnoreReturnValue
public WorkspaceBuilder setPerCommandSyscallCache(SyscallCache perCommandSyscallCache) {
public WorkspaceBuilder setSyscallCache(SyscallCache syscallCache) {
Preconditions.checkState(
this.perCommandSyscallCache == null,
"Set twice: %s %s",
this.perCommandSyscallCache,
perCommandSyscallCache);
this.perCommandSyscallCache = Preconditions.checkNotNull(perCommandSyscallCache);
this.syscallCache == null, "Set twice: %s %s", this.syscallCache, syscallCache);
this.syscallCache = Preconditions.checkNotNull(syscallCache);
return this;
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1814,8 +1814,8 @@ java_library(
)

java_library(
name = "per_build_syscall_cache",
srcs = ["PerBuildSyscallCache.java"],
name = "default_syscall_cache",
srcs = ["DefaultSyscallCache.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/vfs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,15 @@
import javax.annotation.Nullable;

/**
* A per-build cache of filesystem operations.
* A basic implementation of {@link SyscallCache} that caches stat and readdir operations, used if
* no custom cache is set in {@link
* com.google.devtools.build.lib.runtime.WorkspaceBuilder#setSyscallCache}.
*
* <p>Allows non-Skyframe operations (like non-Skyframe globbing) to share a filesystem cache with
* Skyframe operations, and may be able to answer questions (like the type of a file) based on
* existing data (like the directory listing of a parent) without filesystem access.
*/
public final class PerBuildSyscallCache implements SyscallCache {
public final class DefaultSyscallCache implements SyscallCache {
private final Supplier<LoadingCache<Pair<Path, Symlinks>, Object>> statCacheSupplier;
private final Supplier<LoadingCache<Path, Object>> readdirCacheSupplier;

Expand All @@ -47,7 +49,7 @@ public final class PerBuildSyscallCache implements SyscallCache {

private static final FileStatus NO_STATUS = new FakeFileStatus();

private PerBuildSyscallCache(
private DefaultSyscallCache(
Supplier<LoadingCache<Pair<Path, Symlinks>, Object>> statCacheSupplier,
Supplier<LoadingCache<Path, Object>> readdirCacheSupplier) {
this.statCacheSupplier = statCacheSupplier;
Expand Down Expand Up @@ -89,7 +91,7 @@ public Builder setInitialCapacity(int initialCapacity) {
return this;
}

public PerBuildSyscallCache build() {
public DefaultSyscallCache build() {
Caffeine<Object, Object> statCacheBuilder = Caffeine.newBuilder();
if (maxStats != UNSET) {
statCacheBuilder.maximumSize(maxStats);
Expand All @@ -102,9 +104,9 @@ public PerBuildSyscallCache build() {
statCacheBuilder.initialCapacity(initialCapacity);
readdirCacheBuilder.initialCapacity(initialCapacity);
}
return new PerBuildSyscallCache(
() -> statCacheBuilder.build(PerBuildSyscallCache::statImpl),
() -> readdirCacheBuilder.build(PerBuildSyscallCache::readdirImpl));
return new DefaultSyscallCache(
() -> statCacheBuilder.build(DefaultSyscallCache::statImpl),
() -> readdirCacheBuilder.build(DefaultSyscallCache::readdirImpl));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class DirectoryListingStateFunction implements SkyFunction {
private final ExternalFilesHelper externalFilesHelper;

/**
* A file-system abstraction to use. This can e.g. be a {@link PerBuildSyscallCache} which helps
* A file-system abstraction to use. This can e.g. be a {@link DefaultSyscallCache} which helps
* re-use the results of expensive readdir() operations, that are likely already executed for
* evaluating globs.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ private SequencedSkyframeExecutor(
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
SyscallCache perCommandSyscallCache,
SyscallCache syscallCache,
SkyFunction ignoredPackagePrefixesFunction,
CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy,
ImmutableList<BuildFileName> buildFilesByPriority,
Expand All @@ -189,14 +189,14 @@ private SequencedSkyframeExecutor(
actionKeyContext,
workspaceStatusActionFactory,
extraSkyFunctions,
perCommandSyscallCache,
syscallCache,
ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
ignoredPackagePrefixesFunction,
crossRepositoryLabelViolationStrategy,
buildFilesByPriority,
externalPackageHelper,
actionOnIOExceptionReadingBuildFile,
/*shouldUnblockCpuWorkWhenFetchingDeps=*/ false,
/* shouldUnblockCpuWorkWhenFetchingDeps= */ false,
new PackageProgressReceiver(),
new ConfiguredTargetProgressReceiver(),
skyKeyStateReceiver,
Expand Down Expand Up @@ -487,8 +487,7 @@ private void handleDiffsWithMissingDiffInformation(
.build();
memoizingEvaluator.evaluate(ImmutableList.of(), evaluationContext);

FilesystemValueChecker fsvc =
new FilesystemValueChecker(tsgm, perCommandSyscallCache, fsvcThreads);
FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, syscallCache, fsvcThreads);
// We need to manually check for changes to known files. This entails finding all dirty file
// system values under package roots for which we don't have diff information. If at least
// one path entry doesn't have diff information, then we're going to have to iterate over
Expand Down Expand Up @@ -558,8 +557,7 @@ private void handleDiffsWithMissingDiffInformation(
logger.atInfo().log(
"About to scan %d external files",
externalFilesKnowledge.nonOutputExternalFilesSeen.size());
FilesystemValueChecker fsvc =
new FilesystemValueChecker(tsgm, perCommandSyscallCache, fsvcThreads);
FilesystemValueChecker fsvc = new FilesystemValueChecker(tsgm, syscallCache, fsvcThreads);
ImmutableBatchDirtyResult batchDirtyResult;
try (SilentCloseable c = Profiler.instance().profile("fsvc.getDirtyExternalKeys")) {
Map<SkyKey, SkyValue> externalDirtyNodes = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -730,7 +728,7 @@ protected void invalidateFilesUnderPathForTestingImpl(
Differencer.Diff diff;
if (modifiedFileSet.treatEverythingAsModified()) {
diff =
new FilesystemValueChecker(tsgm, perCommandSyscallCache, /*numThreads=*/ 200)
new FilesystemValueChecker(tsgm, syscallCache, /* numThreads= */ 200)
.getDirtyKeys(memoizingEvaluator.getValues(), new BasicFilesystemDirtinessChecker());
} else {
diff = getDiff(tsgm, modifiedFileSet, pathEntry, /* fsvcThreads= */ 200);
Expand All @@ -757,7 +755,7 @@ public void detectModifiedOutputFiles(
long startTime = System.nanoTime();
FilesystemValueChecker fsvc =
new FilesystemValueChecker(
Preconditions.checkNotNull(tsgm.get()), perCommandSyscallCache, fsvcThreads);
Preconditions.checkNotNull(tsgm.get()), syscallCache, fsvcThreads);
BatchStat batchStatter = outputService == null ? null : outputService.getBatchStatter();
recordingDiffer.invalidate(
fsvc.getDirtyActionValues(
Expand Down Expand Up @@ -977,7 +975,7 @@ public static final class Builder {
private SkyFunction ignoredPackagePrefixesFunction;
private BugReporter bugReporter = BugReporter.defaultInstance();
private SkyKeyStateReceiver skyKeyStateReceiver = SkyKeyStateReceiver.NULL_INSTANCE;
private SyscallCache perCommandSyscallCache = null;
private SyscallCache syscallCache = null;

private Builder() {}

Expand All @@ -1004,7 +1002,7 @@ public SequencedSkyframeExecutor build() {
diffAwarenessFactories,
workspaceInfoFromDiffReceiver,
extraSkyFunctions,
Preconditions.checkNotNull(perCommandSyscallCache),
Preconditions.checkNotNull(syscallCache),
ignoredPackagePrefixesFunction,
crossRepositoryLabelViolationStrategy,
buildFilesByPriority,
Expand Down Expand Up @@ -1127,8 +1125,8 @@ public Builder setSkyKeyStateReceiver(SkyKeyStateReceiver skyKeyStateReceiver) {
}

@CanIgnoreReturnValue
public Builder setPerCommandSyscallCache(SyscallCache perCommandSyscallCache) {
this.perCommandSyscallCache = perCommandSyscallCache;
public Builder setSyscallCache(SyscallCache syscallCache) {
this.syscallCache = syscallCache;
return this;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

/** A factory of SkyframeExecutors that returns SequencedSkyframeExecutor. */
public final class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory {

@Override
public SkyframeExecutor create(
PackageFactory pkgFactory,
Expand All @@ -36,7 +37,7 @@ public SkyframeExecutor create(
Factory workspaceStatusActionFactory,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
SyscallCache perCommandSyscallCache,
SyscallCache syscallCache,
@Nullable SkyframeExecutorRepositoryHelpersHolder repositoryHelpersHolder,
SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver,
BugReporter bugReporter) {
Expand All @@ -48,7 +49,7 @@ public SkyframeExecutor create(
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
.setDiffAwarenessFactories(diffAwarenessFactories)
.setExtraSkyFunctions(extraSkyFunctions)
.setPerCommandSyscallCache(perCommandSyscallCache)
.setSyscallCache(syscallCache)
.setRepositoryHelpersHolder(repositoryHelpersHolder)
.setSkyKeyStateReceiver(skyKeyStateReceiver)
.setBugReporter(bugReporter)
Expand Down
Loading

0 comments on commit 8a53b0e

Please sign in to comment.