From b2a943434d413be2e25fbc9cd57a4f84afd6d4c5 Mon Sep 17 00:00:00 2001 From: janakr Date: Sat, 5 Feb 2022 22:02:14 -0800 Subject: [PATCH] Make SyscallCache a constant over the lifetime of the server: it was already effectively a constant, but we initialized it based on the value of --legacy_globbing_threads on the first invocation that did package loading. That never really made much sense (initial capacity != concurrency) and anyway with Skyframe --legacy_globbing_threads isn't the controlling knob for concurrent accesses to the SyscallCache, since it's accessed directly in Skyframe. Moreover, the memory cost of an overly large initial capacity is negligible (8 bytes per entry), so we just change it to cost (JVM heap)/1M. It was a mistake to notice this only after https://github.com/bazelbuild/bazel/commit/491e4418c225d224466c2211f608b42241c6c55a: this changes many of the same lines. PiperOrigin-RevId: 426688249 --- .../java/com/google/devtools/build/lib/BUILD | 1 + .../build/lib/packages/PackageFactory.java | 11 ++-- .../build/lib/runtime/BlazeWorkspace.java | 7 ++- .../build/lib/runtime/CommandEnvironment.java | 3 +- .../build/lib/runtime/WorkspaceBuilder.java | 45 +++++++++++++- .../build/lib/skyframe/ArtifactFunction.java | 7 +-- .../DirectoryListingStateFunction.java | 7 +-- .../build/lib/skyframe/FileStateFunction.java | 6 +- .../lib/skyframe/PerBuildSyscallCache.java | 5 +- .../RecursiveFilesystemTraversalFunction.java | 10 ++-- .../skyframe/SequencedSkyframeExecutor.java | 11 +++- .../SequencedSkyframeExecutorFactory.java | 3 + .../lib/skyframe/SkyframeActionExecutor.java | 8 +-- .../build/lib/skyframe/SkyframeExecutor.java | 59 +++++-------------- .../lib/skyframe/SkyframeExecutorFactory.java | 2 + .../lib/skyframe/SkyframePackageManager.java | 6 +- .../packages/AbstractPackageLoader.java | 4 +- .../skyframe/packages/BazelPackageLoader.java | 3 +- .../devtools/build/lib/vfs/SyscallCache.java | 14 ++++- .../lib/analysis/util/AnalysisTestCase.java | 2 + .../lib/analysis/util/BuildViewTestCase.java | 3 +- .../analysis/util/ConfigurationTestCase.java | 2 + .../bzlmod/BzlmodRepoRuleFunctionTest.java | 2 +- .../bzlmod/BzlmodRepoRuleHelperTest.java | 2 +- .../build/lib/bazel/bzlmod/DiscoveryTest.java | 2 +- .../bzlmod/ModuleExtensionResolutionTest.java | 2 +- .../bazel/bzlmod/ModuleFileFunctionTest.java | 2 +- .../packages/util/PackageLoadingTestCase.java | 2 + .../pkgcache/BuildFileModificationTest.java | 2 + .../lib/pkgcache/IncrementalLoadingTest.java | 2 + .../lib/pkgcache/LoadingPhaseRunnerTest.java | 2 + .../lib/pkgcache/PackageLoadingTest.java | 2 + .../TargetPatternEvaluatorIOTest.java | 3 - .../testutil/PostAnalysisQueryTest.java | 40 ++----------- .../query2/testutil/SkyframeQueryHelper.java | 2 + .../repository/ExternalPackageHelperTest.java | 2 +- .../repository/RepositoryDelegatorTest.java | 2 +- .../build/lib/runtime/BlazeRuntimeTest.java | 3 + ...ractCollectPackagesUnderDirectoryTest.java | 2 + .../skyframe/ArtifactFunctionTestCase.java | 4 +- .../ContainingPackageLookupFunctionTest.java | 4 +- .../build/lib/skyframe/FileFunctionTest.java | 2 +- .../skyframe/FilesetEntryFunctionTest.java | 6 +- .../skyframe/FilesystemValueCheckerTest.java | 2 +- .../build/lib/skyframe/GlobFunctionTest.java | 4 +- .../LocalRepositoryLookupFunctionTest.java | 4 +- .../lib/skyframe/PackageFunctionTest.java | 7 --- .../skyframe/PackageLookupFunctionTest.java | 4 +- .../PathCasingLookupFunctionTest.java | 4 +- ...psOfPatternsFunctionSmartNegationTest.java | 2 + ...ursiveFilesystemTraversalFunctionTest.java | 6 +- .../skyframe/TimestampBuilderTestCase.java | 7 +-- .../skyframe/WorkspaceFileFunctionTest.java | 6 -- .../devtools/build/lib/vfs/GlobTest.java | 12 +++- 54 files changed, 199 insertions(+), 168 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 86c7ce8ade913f..cff07dba74b4b0 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -329,6 +329,7 @@ java_library( "//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:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 5edd7ac182a829..1a15703fd4d4a4 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -54,7 +54,6 @@ import java.util.Set; import java.util.concurrent.ForkJoinPool; import java.util.function.Consumer; -import java.util.function.Supplier; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Module; @@ -114,7 +113,7 @@ public interface EnvironmentExtension { private final RuleFactory ruleFactory; private final RuleClassProvider ruleClassProvider; - private Supplier syscalls; + private SyscallCache syscallCache; private ForkJoinPool executor; @@ -210,9 +209,9 @@ public PackageFactory( version); } - /** Sets the syscalls cache used in globbing. */ - public void setSyscallCache(Supplier syscalls) { - this.syscalls = Preconditions.checkNotNull(syscalls); + /** Sets the syscalls cache used in filesystem access. */ + public void setSyscallCache(SyscallCache syscallCache) { + this.syscallCache = Preconditions.checkNotNull(syscallCache); } /** @@ -481,7 +480,7 @@ public NonSkyframeGlobber createNonSkyframeGlobber( packageId, ignoredGlobPrefixes, locator, - syscalls.get(), + syscallCache, executor, maxDirectoriesToEagerlyVisitInGlobbing, threadStateReceiverForMetrics)); diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java index 5a13e68f0e462e..5eb00a6048fb79 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.common.options.OptionsParsingResult; import com.google.protobuf.Any; import java.io.IOException; @@ -62,6 +63,7 @@ public final class BlazeWorkspace { private final BlazeDirectories directories; private final SkyframeExecutor skyframeExecutor; + private final SyscallCache perCommandSyscallCache; /** * Loaded lazily on the first build command that enables the action cache. Cleared on a build @@ -81,7 +83,8 @@ public BlazeWorkspace( SubscriberExceptionHandler eventBusExceptionHandler, WorkspaceStatusAction.Factory workspaceStatusActionFactory, BinTools binTools, - @Nullable AllocationTracker allocationTracker) { + @Nullable AllocationTracker allocationTracker, + SyscallCache perCommandSyscallCache) { this.runtime = runtime; this.eventBusExceptionHandler = Preconditions.checkNotNull(eventBusExceptionHandler); this.workspaceStatusActionFactory = workspaceStatusActionFactory; @@ -90,6 +93,7 @@ public BlazeWorkspace( this.directories = directories; this.skyframeExecutor = skyframeExecutor; + this.perCommandSyscallCache = perCommandSyscallCache; if (directories.inWorkspace()) { writeOutputBaseReadmeFile(); @@ -207,6 +211,7 @@ public CommandEnvironment initCommand( Thread.currentThread(), command, options, + perCommandSyscallCache, warnings, waitTimeInMs, commandStartTime, diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index de0b296ea2edd0..1781a6f79a9e19 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -161,6 +161,7 @@ public void exit(AbruptExitException exception) { Thread commandThread, Command command, OptionsParsingResult options, + SyscallCache syscallCache, List warnings, long waitTimeInMs, long commandStartTime, @@ -175,6 +176,7 @@ public void exit(AbruptExitException exception) { this.command = command; this.options = options; this.shutdownReasonConsumer = shutdownReasonConsumer; + this.syscallCache = syscallCache; this.blazeModuleEnvironment = new BlazeModuleEnvironment(); this.timestampGranularityMonitor = new TimestampGranularityMonitor(runtime.getClock()); // Record the command's starting time again, for use by @@ -220,7 +222,6 @@ public void exit(AbruptExitException exception) { } else { this.packageLocator = null; } - this.syscallCache = workspace.getSkyframeExecutor().getCurrentSyscallCache(); workspace.getSkyframeExecutor().setEventBus(eventBus); ClientOptions clientOptions = diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java index 84a732a24fc8f2..652444f679a6fb 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java @@ -19,15 +19,18 @@ import com.google.common.eventbus.SubscriberExceptionHandler; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction; +import com.google.devtools.build.lib.bugreport.BugReport; 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.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; import com.google.devtools.build.lib.skyframe.SkyframeExecutorRepositoryHelpersHolder; import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; import java.util.Map; @@ -55,12 +58,34 @@ public final class WorkspaceBuilder { private SkyframeExecutorRepositoryHelpersHolder skyframeExecutorRepositoryHelpersHolder = null; @Nullable private SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver = null; + private SyscallCache perCommandSyscallCache; WorkspaceBuilder(BlazeDirectories directories, BinTools binTools) { this.directories = directories; this.binTools = binTools; } + public static int getSyscallCacheInitialCapacity() { + // The initial capacity here translates into the size of an array in ConcurrentHashMap, so + // oversizing by N results in memory usage of 8N bytes. So the maximum wasted memory here is + // 1/2^20 of heap, or 10K on a 10G heap (which would start with 1280-capacity caches). + long scaledMemory = Runtime.getRuntime().maxMemory() >> 23; + if (scaledMemory > Integer.MAX_VALUE) { + // Something went very wrong. + BugReport.sendBugReport( + new IllegalStateException( + "Scaled memory was still too big: " + + scaledMemory + + ", " + + Runtime.getRuntime().maxMemory())); + scaledMemory = 1024; + } else if (scaledMemory <= 0) { + // If Bazel is running in <8M of memory, very impressive. + scaledMemory = 32; + } + return (int) scaledMemory; + } + BlazeWorkspace build( BlazeRuntime runtime, PackageFactory packageFactory, @@ -69,6 +94,12 @@ BlazeWorkspace build( if (skyframeExecutorFactory == null) { skyframeExecutorFactory = new SequencedSkyframeExecutorFactory(); } + if (perCommandSyscallCache == null) { + perCommandSyscallCache = + PerBuildSyscallCache.newBuilder() + .setInitialCapacity(getSyscallCacheInitialCapacity()) + .build(); + } SkyframeExecutor skyframeExecutor = skyframeExecutorFactory.create( @@ -79,6 +110,7 @@ BlazeWorkspace build( workspaceStatusActionFactory, diffAwarenessFactories.build(), skyFunctions.buildOrThrow(), + perCommandSyscallCache, skyframeExecutorRepositoryHelpersHolder, skyKeyStateReceiver == null ? SkyframeExecutor.SkyKeyStateReceiver.NULL_INSTANCE @@ -91,7 +123,8 @@ BlazeWorkspace build( eventBusExceptionHandler, workspaceStatusActionFactory, binTools, - allocationTracker); + allocationTracker, + perCommandSyscallCache); } /** @@ -127,6 +160,16 @@ public WorkspaceBuilder setAllocationTracker(AllocationTracker allocationTracker return this; } + public WorkspaceBuilder setPerCommandSyscallCache(SyscallCache perCommandSyscallCache) { + Preconditions.checkState( + this.perCommandSyscallCache == null, + "Set twice: %s %s", + this.perCommandSyscallCache, + perCommandSyscallCache); + this.perCommandSyscallCache = Preconditions.checkNotNull(perCommandSyscallCache); + return this; + } + /** * Add a {@link DiffAwareness} factory. These will be used to determine which files, if any, * changed between Blaze commands. Note that these factories are attempted in the order in which diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java index f170fee5e24458..b8d262a4dbe680 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java @@ -69,7 +69,7 @@ class ArtifactFunction implements SkyFunction { private final Supplier mkdirForTreeArtifacts; private final MetadataConsumerForMetrics sourceArtifactsSeen; - private final Supplier syscallCache; + private final SyscallCache syscallCache; static final class MissingArtifactValue implements SkyValue { private final DetailedExitCode detailedExitCode; @@ -96,7 +96,7 @@ public String toString() { public ArtifactFunction( Supplier mkdirForTreeArtifacts, MetadataConsumerForMetrics sourceArtifactsSeen, - Supplier syscallCache) { + SyscallCache syscallCache) { this.mkdirForTreeArtifacts = mkdirForTreeArtifacts; this.sourceArtifactsSeen = sourceArtifactsSeen; this.syscallCache = syscallCache; @@ -279,8 +279,7 @@ private SkyValue createSourceValue(Artifact artifact, Environment env) if (!fileValue.isDirectory() || !TrackSourceDirectoriesFlag.trackSourceDirectories()) { FileArtifactValue metadata; try { - metadata = - FileArtifactValue.createForSourceArtifact(artifact, fileValue, syscallCache.get()); + metadata = FileArtifactValue.createForSourceArtifact(artifact, fileValue, syscallCache); } catch (IOException e) { throw new ArtifactFunctionException( SourceArtifactException.create(artifact, e), Transience.TRANSIENT); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java index 5df2ca81679fb8..48b47a2a0dbb57 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java @@ -20,7 +20,6 @@ import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; import java.io.IOException; -import java.util.function.Supplier; /** * A {@link SkyFunction} for {@link DirectoryListingStateValue}s. @@ -37,10 +36,10 @@ public class DirectoryListingStateFunction implements SkyFunction { * re-use the results of expensive readdir() operations, that are likely already executed for * evaluating globs. */ - private final Supplier syscallCache; + private final SyscallCache syscallCache; public DirectoryListingStateFunction( - ExternalFilesHelper externalFilesHelper, Supplier syscallCache) { + ExternalFilesHelper externalFilesHelper, SyscallCache syscallCache) { this.externalFilesHelper = externalFilesHelper; this.syscallCache = syscallCache; } @@ -62,7 +61,7 @@ public DirectoryListingStateValue compute(SkyKey skyKey, Environment env) // the file system is frozen at the beginning of the build command. return DirectoryListingStateValue.create(dirRootedPath); } - return DirectoryListingStateValue.create(syscallCache.get().readdir(dirRootedPath.asPath())); + return DirectoryListingStateValue.create(syscallCache.readdir(dirRootedPath.asPath())); } catch (ExternalFilesHelper.NonexistentImmutableExternalFileException e) { // DirectoryListingStateValue.key assumes the path exists. This exception here is therefore // indicative of a programming bug. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java index 41399fc07deaa9..c16fa1d4a35c5e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java @@ -34,12 +34,12 @@ public class FileStateFunction implements SkyFunction { private final Supplier tsgm; - private final Supplier syscallCache; + private final SyscallCache syscallCache; private final ExternalFilesHelper externalFilesHelper; public FileStateFunction( Supplier tsgm, - Supplier syscallCache, + SyscallCache syscallCache, ExternalFilesHelper externalFilesHelper) { this.tsgm = tsgm; this.syscallCache = syscallCache; @@ -65,7 +65,7 @@ public FileStateValue compute(SkyKey skyKey, Environment env) // do not use syscallCache as files under repositories get generated during the build return FileStateValue.create(rootedPath, tsgm.get()); } - return FileStateValue.create(rootedPath, syscallCache.get(), tsgm.get()); + return FileStateValue.create(rootedPath, syscallCache, tsgm.get()); } catch (ExternalFilesHelper.NonexistentImmutableExternalFileException e) { return FileStateValue.NONEXISTENT_FILE_STATE_NODE; } catch (InconsistentFilesystemException e) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java index b9e9af8c3d3c02..cd3bc3d5f0e4d2 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java @@ -29,7 +29,9 @@ /** * A per-build cache of filesystem operations. * - *

Mostly used by non-Skyframe globbing and include parsing. + *

Allows non-Skyframe operations (like legacy globbing) to share a filesystem cache with + * Skyframe nodes, 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 { @@ -181,6 +183,7 @@ public Dirent.Type getType(Path path, Symlinks symlinks) throws IOException { return SyscallCache.statusToDirentType(statIfFound(path, symlinks)); } + @Override public void clear() { statCache.invalidateAll(); readdirCache.invalidateAll(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index 12f56ec6ff6b39..26ffd612e1c41c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java @@ -66,7 +66,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.function.Supplier; import javax.annotation.Nullable; /** A {@link SkyFunction} to build {@link RecursiveFilesystemTraversalValue}s. */ @@ -145,9 +144,9 @@ private static final class RecursiveFilesystemTraversalFunctionException extends } } - private final Supplier syscallCache; + private final SyscallCache syscallCache; - RecursiveFilesystemTraversalFunction(Supplier syscallCache) { + RecursiveFilesystemTraversalFunction(SyscallCache syscallCache) { this.syscallCache = syscallCache; } @@ -160,7 +159,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) Profiler.instance() .profile(ProfilerTask.FILESYSTEM_TRAVERSAL, traversal.getRoot().toString())) { // Stat the traversal root. - FileInfo rootInfo = lookUpFileInfo(env, traversal, syscallCache.get()); + FileInfo rootInfo = lookUpFileInfo(env, traversal, syscallCache); if (rootInfo == null) { return null; } @@ -195,8 +194,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) } // Otherwise the root is a directory or a symlink to one. - PkgLookupResult pkgLookupResult = - checkIfPackage(env, traversal, rootInfo, syscallCache.get()); + PkgLookupResult pkgLookupResult = checkIfPackage(env, traversal, rootInfo, syscallCache); if (pkgLookupResult == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 36e92cf211e533..570808d5b05789 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -90,6 +90,7 @@ import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.Differencer; import com.google.devtools.build.skyframe.EvaluationContext; import com.google.devtools.build.skyframe.EventFilter; @@ -166,6 +167,7 @@ private SequencedSkyframeExecutor( Iterable diffAwarenessFactories, WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver, ImmutableMap extraSkyFunctions, + SyscallCache perCommandSyscallCache, SkyFunction ignoredPackagePrefixesFunction, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, ImmutableList buildFilesByPriority, @@ -182,6 +184,7 @@ private SequencedSkyframeExecutor( actionKeyContext, workspaceStatusActionFactory, extraSkyFunctions, + perCommandSyscallCache, ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, ignoredPackagePrefixesFunction, crossRepositoryLabelViolationStrategy, @@ -771,7 +774,6 @@ protected void invalidateFilesUnderPathForTestingImpl( throws InterruptedException, AbruptExitException { TimestampGranularityMonitor tsgm = this.tsgm.get(); Differencer.Diff diff; - syscallCacheRef.set(getPerBuildSyscallCache(/*globbingThreads=*/ 42)); if (modifiedFileSet.treatEverythingAsModified()) { diff = new FilesystemValueChecker(tsgm, /*lastExecutionTimeRange=*/ null, /*numThreads=*/ 200) @@ -1091,6 +1093,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 Builder() {} @@ -1117,6 +1120,7 @@ public SequencedSkyframeExecutor build() { diffAwarenessFactories, workspaceInfoFromDiffReceiver, extraSkyFunctions, + Preconditions.checkNotNull(perCommandSyscallCache), ignoredPackagePrefixesFunction, crossRepositoryLabelViolationStrategy, buildFilesByPriority, @@ -1220,5 +1224,10 @@ public Builder setSkyKeyStateReceiver(SkyKeyStateReceiver skyKeyStateReceiver) { this.skyKeyStateReceiver = Preconditions.checkNotNull(skyKeyStateReceiver); return this; } + + public Builder setPerCommandSyscallCache(SyscallCache perCommandSyscallCache) { + this.perCommandSyscallCache = perCommandSyscallCache; + return this; + } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java index b7191470bc0054..099db51a92a8cd 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; import javax.annotation.Nullable; @@ -35,6 +36,7 @@ public SkyframeExecutor create( Factory workspaceStatusActionFactory, Iterable diffAwarenessFactories, ImmutableMap extraSkyFunctions, + SyscallCache perCommandSyscallCache, @Nullable SkyframeExecutorRepositoryHelpersHolder repositoryHelpersHolder, SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver, BugReporter bugReporter) { @@ -46,6 +48,7 @@ public SkyframeExecutor create( .setWorkspaceStatusActionFactory(workspaceStatusActionFactory) .setDiffAwarenessFactories(diffAwarenessFactories) .setExtraSkyFunctions(extraSkyFunctions) + .setPerCommandSyscallCache(perCommandSyscallCache) .setRepositoryHelpersHolder(repositoryHelpersHolder) .setSkyKeyStateReceiver(skyKeyStateReceiver) .setBugReporter(bugReporter) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 111b866867e5d4..82adf9d8ccd454 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -146,7 +146,7 @@ public void injectTree(SpecialArtifact output, TreeArtifactValue tree) { private final ActionKeyContext actionKeyContext; private final MetadataConsumerForMetrics outputArtifactsSeen; private final MetadataConsumerForMetrics outputArtifactsFromActionCache; - private final Supplier syscallCache; + private final SyscallCache syscallCache; private final Function threadStateReceiverFactory; private Reporter reporter; private Map clientEnv = ImmutableMap.of(); @@ -218,7 +218,7 @@ public void injectTree(SpecialArtifact output, TreeArtifactValue tree) { MetadataConsumerForMetrics outputArtifactsFromActionCache, AtomicReference statusReporterRef, Supplier> sourceRootSupplier, - Supplier syscallCache, + SyscallCache syscallCache, Function threadStateReceiverFactory) { this.actionKeyContext = actionKeyContext; this.outputArtifactsSeen = outputArtifactsSeen; @@ -546,7 +546,7 @@ private ActionExecutionContext getContext( actionFileSystem, skyframeDepsResult, discoveredModulesPruner, - syscallCache.get(), + syscallCache, threadStateReceiverFactory.apply(actionLookupData)); } @@ -767,7 +767,7 @@ NestedSet discoverInputs( env, actionFileSystem, discoveredModulesPruner, - syscallCache.get(), + syscallCache, threadStateReceiverFactory.apply(actionLookupData)); if (actionFileSystem != null) { updateActionFileSystemContext( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 7a7af2bf918b0f..b0a4f37a02faa9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -286,6 +286,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur private final AtomicInteger numPackagesLoaded = new AtomicInteger(0); @Nullable private final PackageProgressReceiver packageProgress; @Nullable private final ConfiguredTargetProgressReceiver configuredTargetProgress; + private final SyscallCache perCommandSyscallCache; private final SkyframeBuildView skyframeBuildView; private ActionLogBufferPathGenerator actionLogBufferPathGenerator; @@ -294,8 +295,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur // AtomicReferences are used here as mutable boxes shared with value builders. private final AtomicBoolean showLoadingProgress = new AtomicBoolean(); - protected final AtomicReference syscallCacheRef = - new AtomicReference<>(SyscallCache.NO_CACHE); protected final AtomicReference pkgLocator = new AtomicReference<>(); protected final AtomicReference> deletedPackages = new AtomicReference<>(ImmutableSet.of()); @@ -354,8 +353,6 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur private final SkyKeyStateReceiver skyKeyStateReceiver; - private PerBuildSyscallCache perBuildSyscallCache; - private final PathResolverFactory pathResolverFactory = new PathResolverFactoryImpl(); private boolean siblingRepositoryLayout = false; @@ -403,6 +400,7 @@ protected SkyframeExecutor( ActionKeyContext actionKeyContext, Factory workspaceStatusActionFactory, ImmutableMap extraSkyFunctions, + SyscallCache perCommandSyscallCache, ExternalFileAction externalFileAction, SkyFunction ignoredPackagePrefixesFunction, CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, @@ -422,14 +420,15 @@ protected SkyframeExecutor( this.shouldUnblockCpuWorkWhenFetchingDeps = shouldUnblockCpuWorkWhenFetchingDeps; this.skyKeyStateReceiver = skyKeyStateReceiver; this.bugReporter = bugReporter; - this.pkgFactory.setSyscallCache(syscallCacheRef::get); + this.perCommandSyscallCache = perCommandSyscallCache; + this.pkgFactory.setSyscallCache(this.perCommandSyscallCache); this.workspaceStatusActionFactory = workspaceStatusActionFactory; this.packageManager = new SkyframePackageManager( new SkyframePackageLoader(), new QueryTransitivePackagePreloader( () -> memoizingEvaluator, this::newEvaluationContextBuilder), - syscallCacheRef::get, + this.perCommandSyscallCache, pkgLocator::get, numPackagesLoaded); this.fileSystem = fileSystem; @@ -446,7 +445,7 @@ protected SkyframeExecutor( outputArtifactsFromActionCache, statusReporterRef, this::getPathEntries, - syscallCacheRef::get, + this.perCommandSyscallCache, skyKeyStateReceiver::makeThreadStateReceiver); this.artifactFactory = new ArtifactFactory( @@ -600,7 +599,7 @@ private ImmutableMap skyFunctions() { new ArtifactFunction( () -> !skyframeActionExecutor.actionFileSystemType().inMemoryFileSystem(), sourceArtifactsSeen, - syscallCacheRef::get)); + perCommandSyscallCache)); map.put( SkyFunctions.BUILD_INFO_COLLECTION, new BuildInfoCollectionFunction(actionKeyContext, artifactFactory)); @@ -612,7 +611,7 @@ private ImmutableMap skyFunctions() { this.actionExecutionFunction = actionExecutionFunction; map.put( SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, - new RecursiveFilesystemTraversalFunction(syscallCacheRef::get)); + new RecursiveFilesystemTraversalFunction(perCommandSyscallCache)); map.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction(directories::getExecRoot)); map.put( SkyFunctions.ACTION_TEMPLATE_EXPANSION, @@ -653,11 +652,11 @@ protected boolean traverseTestSuites() { } protected SkyFunction newFileStateFunction() { - return new FileStateFunction(tsgm::get, syscallCacheRef::get, externalFilesHelper); + return new FileStateFunction(tsgm::get, perCommandSyscallCache, externalFilesHelper); } protected SkyFunction newDirectoryListingStateFunction() { - return new DirectoryListingStateFunction(externalFilesHelper, syscallCacheRef::get); + return new DirectoryListingStateFunction(externalFilesHelper, perCommandSyscallCache); } protected GlobFunction newGlobFunction() { @@ -687,25 +686,6 @@ protected PerBuildSyscallCache newPerBuildSyscallCache(int globbingThreads) { return PerBuildSyscallCache.newBuilder().setInitialCapacity(globbingThreads).build(); } - /** - * Gets a (possibly cached) syscalls cache, re-initialized each build. - * - *

We cache the syscalls cache if possible because construction of the cache is surprisingly - * expensive, and is on the critical path of null builds. - */ - protected final PerBuildSyscallCache getPerBuildSyscallCache(int globbingThreads) { - if (perBuildSyscallCache == null) { - perBuildSyscallCache = newPerBuildSyscallCache(globbingThreads); - } else { - perBuildSyscallCache.clear(); - } - return perBuildSyscallCache; - } - - public final SyscallCache getCurrentSyscallCache() { - return syscallCacheRef.get(); - } - @ThreadCompatible public void setActive(boolean active) { this.active = active; @@ -1367,7 +1347,7 @@ public void preparePackageLoading( starlarkSemantics.getBool(BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT)); setPackageLocator(pkgLocator); - syscallCacheRef.set(getPerBuildSyscallCache(packageOptions.globbingThreads)); + perCommandSyscallCache.clear(); this.pkgFactory.setGlobbingThreads(packageOptions.globbingThreads); this.pkgFactory.setMaxDirectoriesToEagerlyVisitInGlobbing( packageOptions.maxDirectoriesToEagerlyVisitInGlobbing); @@ -2243,14 +2223,10 @@ public final void invalidateFilesUnderPathForTesting( dropConfiguredTargetsNow(eventHandler); lastAnalysisDiscarded = false; } + perCommandSyscallCache.clear(); invalidateFilesUnderPathForTestingImpl(eventHandler, modifiedFileSet, pathEntry); } - @VisibleForTesting - public final void turnOffSyscallCacheForTesting() { - syscallCacheRef.set(SyscallCache.NO_CACHE); - } - protected abstract void invalidateFilesUnderPathForTestingImpl( ExtendedEventHandler eventHandler, ModifiedFileSet modifiedFileSet, Root pathEntry) throws InterruptedException, AbruptExitException; @@ -2282,8 +2258,7 @@ EvaluationResult configureTargets( EvaluationResult result = memoizingEvaluator.evaluate( Iterables.concat(configuredTargetKeys, topLevelAspectKeys), evaluationContext); - // Get rid of any memory retained by the cache -- all loading is done. - perBuildSyscallCache.clear(); + perCommandSyscallCache.noteAnalysisPhaseEnded(); return result; } @@ -2313,12 +2288,8 @@ EvaluationResult evaluateBuildDriverKeys( .setExecutionPhaseThreadPoolSize(mergedPhasesExecutionJobsCount) .setEventHandler(eventHandler) .build(); - EvaluationResult result = - memoizingEvaluator.evaluate( - Iterables.concat(buildDriverCTKeys, buildDriverAspectKeys), evaluationContext); - // Get rid of any memory retained by the cache -- all loading is done. - perBuildSyscallCache.clear(); - return result; + return memoizingEvaluator.evaluate( + Iterables.concat(buildDriverCTKeys, buildDriverAspectKeys), evaluationContext); } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java index 20a26b4fbbd522..3cc6521914c036 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; @@ -49,6 +50,7 @@ SkyframeExecutor create( Factory workspaceStatusActionFactory, Iterable diffAwarenessFactories, ImmutableMap extraSkyFunctions, + SyscallCache perCommandSyscallCache, SkyframeExecutorRepositoryHelpersHolder repositoryHelpersHolder, SkyframeExecutor.SkyKeyStateReceiver skyKeyStateReceiver, BugReporter bugReporter) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java index 2b78b67107f229..3e984d50a48144 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java @@ -40,14 +40,14 @@ class SkyframePackageManager implements PackageManager, CachingPackageLocator { private final SkyframePackageLoader packageLoader; private final QueryTransitivePackagePreloader transitiveLoader; - private final Supplier syscallCache; + private final SyscallCache syscallCache; private final Supplier pkgLocator; private final AtomicInteger numPackagesLoaded; public SkyframePackageManager( SkyframePackageLoader packageLoader, QueryTransitivePackagePreloader transitiveLoader, - Supplier syscallCache, + SyscallCache syscallCache, Supplier pkgLocator, AtomicInteger numPackagesLoaded) { this.packageLoader = packageLoader; @@ -99,7 +99,7 @@ public Path getBuildFileForPackage(PackageIdentifier packageName) { // TODO(bazel-team): Use a PackageLookupValue here [skyframe-loading] // TODO(bazel-team): The implementation in PackageCache also checks for duplicate packages, see // BuildFileCache#getBuildFile [skyframe-loading] - return pkgLocator.get().getPackageBuildFileNullable(packageName, syscallCache.get()); + return pkgLocator.get().getPackageBuildFileNullable(packageName, syscallCache); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 1f13bc1b48a9bb..62b9fe230efc8c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -437,7 +437,7 @@ private ImmutableMap makeFreshSkyFunctions() { TimestampGranularityMonitor tsgm = new TimestampGranularityMonitor(BlazeClock.instance()); PerBuildSyscallCache syscallCache = PerBuildSyscallCache.newBuilder().setInitialCapacity(nonSkyframeGlobbingThreads).build(); - pkgFactory.setSyscallCache(() -> syscallCache); + pkgFactory.setSyscallCache(syscallCache); pkgFactory.setMaxDirectoriesToEagerlyVisitInGlobbing( MAX_DIRECTORIES_TO_EAGERLY_VISIT_IN_GLOBBING); CachingPackageLocator cachingPackageLocator = @@ -459,7 +459,7 @@ public String getBaseNameForLoadedPackage(PackageIdentifier packageName) { .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) .put( FileStateValue.FILE_STATE, - new FileStateFunction(() -> tsgm, () -> syscallCache, externalFilesHelper)) + new FileStateFunction(() -> tsgm, syscallCache, externalFilesHelper)) .put(FileSymlinkCycleUniquenessFunction.NAME, new FileSymlinkCycleUniquenessFunction()) .put( FileSymlinkInfiniteExpansionUniquenessFunction.NAME, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java index bacc17bb2dc304..3cff84dd4d8939 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java @@ -116,8 +116,7 @@ public BazelPackageLoader buildImpl() { new ClientEnvironmentFunction(new AtomicReference<>(ImmutableMap.of()))) .put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction( - externalFilesHelper, () -> SyscallCache.NO_CACHE)) + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)) .put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction()) .put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()) .put( diff --git a/src/main/java/com/google/devtools/build/lib/vfs/SyscallCache.java b/src/main/java/com/google/devtools/build/lib/vfs/SyscallCache.java index a9d02641586c71..aa90b278caccc7 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/SyscallCache.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/SyscallCache.java @@ -39,6 +39,9 @@ public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException { public Dirent.Type getType(Path path, Symlinks symlinks) throws IOException { return statusToDirentType(statIfFound(path, symlinks)); } + + @Override + public void clear() {} }; /** Gets directory entries and their types. Does not follow symlinks. */ @@ -57,8 +60,15 @@ default byte[] getFastDigest(Path path) throws IOException { return path.getFastDigest(); } - default byte[] getDigest(Path path) throws IOException { - return path.getDigest(); + /** Called before each build. Implementations should flush their caches at that point. */ + void clear(); + + /** + * Called at the end of the analysis phase (if not doing merged analysis/execution). Cache may + * choose to drop some data then. + */ + default void noteAnalysisPhaseEnded() { + clear(); } static Dirent.Type statusToDirentType(FileStatus status) { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index 9d0e4308e5c58d..869317ad3303a6 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -86,6 +86,7 @@ import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParser; import java.util.Arrays; @@ -195,6 +196,7 @@ protected SkyframeExecutor createSkyframeExecutor(PackageFactory pkgFactory) { .setActionKeyContext(actionKeyContext) .setWorkspaceStatusActionFactory(workspaceStatusActionFactory) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 5aea39b1f486c7..4f7519ba259ae3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -313,7 +313,8 @@ RepositoryDelegatorFunction.ENABLE_BZLMOD, enableBzlmod())) .setDirectories(directories) .setActionKeyContext(actionKeyContext) .setWorkspaceStatusActionFactory(workspaceStatusActionFactory) - .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)); + .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE); ManagedDirectoriesKnowledge managedDirectoriesKnowledge = getManagedDirectoriesKnowledge(); if (managedDirectoriesKnowledge != null) { builder.setRepositoryHelpersHolder( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java index 3de28af4d2a0f7..025ef144e80a82 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java @@ -50,6 +50,7 @@ import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; @@ -129,6 +130,7 @@ public final void initializeSkyframeExecutor() throws Exception { .setActionKeyContext(actionKeyContext) .setWorkspaceStatusActionFactory(workspaceStatusActionFactory) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); SkyframeExecutorTestHelper.process(skyframeExecutor); skyframeExecutor.injectExtraPrecomputedValues( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java index f8cf52b27a52a2..e00093aca72e03 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java @@ -134,7 +134,7 @@ public void setup() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java index 91f4971eec4a33..c42af7419accea 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperTest.java @@ -111,7 +111,7 @@ public void setup() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put( SkyFunctions.MODULE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java index 6222f985867ae2..90977d22dfdc34 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java @@ -159,7 +159,7 @@ public void setup() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put(DiscoveryValue.FUNCTION_NAME, new DiscoveryFunction()) .put( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index bc8bb1509636bc..5a0532d9481580 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -162,7 +162,7 @@ public void setup() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put( SkyFunctions.MODULE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 27b35c7d5939f1..b5fe9266865781 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -130,7 +130,7 @@ public void setup() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put( SkyFunctions.MODULE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java index e5f48c5f4a52fe..2f78425878fc3d 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParser; import java.util.List; @@ -125,6 +126,7 @@ private SkyframeExecutor createSkyframeExecutor() { .setFileSystem(fileSystem) .setDirectories(directories) .setActionKeyContext(actionKeyContext) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); skyframeExecutor.injectExtraPrecomputedValues( ImmutableList.of( diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java index 26b4aefd2db0e5..48266869ca21a8 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.common.options.OptionsParser; import java.nio.charset.StandardCharsets; @@ -91,6 +92,7 @@ public final void initializeSkyframeExecutor() { .setDirectories(directories) .setActionKeyContext(actionKeyContext) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); skyframeExecutor.injectExtraPrecomputedValues( ImmutableList.of( diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java index ecf83c01ef69fb..5353c64f19fca8 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java @@ -56,6 +56,7 @@ 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 com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsProvider; @@ -450,6 +451,7 @@ public DiffAwareness maybeCreate(Root pathEntry) { .setDirectories(directories) .setActionKeyContext(new ActionKeyContext()) .setDiffAwarenessFactories(ImmutableList.of(new ManualDiffAwarenessFactory())) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); SkyframeExecutorTestHelper.process(skyframeExecutor); PackageOptions packageOptions = Options.getDefaults(PackageOptions.class); diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index eaac956da2ec72..38a060cf4bb995 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -67,6 +67,7 @@ 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 com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParser; @@ -1369,6 +1370,7 @@ private static final class LoadingPhaseTester { .setDirectories(directories) .setActionKeyContext(new ActionKeyContext()) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); SkyframeExecutorTestHelper.process(skyframeExecutor); PathPackageLocator pkgLocator = diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageLoadingTest.java index 92022e2d9ca79d..f3e61aa9ec1dd0 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageLoadingTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageLoadingTest.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.vfs.ModifiedFileSet; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.common.options.OptionsParser; import java.io.IOException; import java.util.Optional; @@ -95,6 +96,7 @@ private void initializeSkyframeExecutor(boolean doPackageLoadingChecks) throws E .setDirectories(directories) .setActionKeyContext(actionKeyContext) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); SkyframeExecutorTestHelper.process(skyframeExecutor); setUpSkyframe(parsePackageOptions(), parseBuildLanguageOptions()); diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorIOTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorIOTest.java index 992638fa490be1..25369bd5824bc0 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorIOTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorIOTest.java @@ -102,7 +102,6 @@ protected Collection readdir(PathFragment path, boolean followSymlinks) @Test public void testBadStat(@TestParameter boolean keepGoing) throws Exception { reporter.removeHandler(failFastHandler); - getSkyframeExecutor().turnOffSyscallCacheForTesting(); // Given a package, "parent", Path parent = scratch.file("parent/BUILD", "sh_library(name = 'parent')").getParentDirectory(); // And a child, "badstat", @@ -130,7 +129,6 @@ public void testBadStat(@TestParameter boolean keepGoing) throws Exception { @Test public void testBadStatPathAsTarget(@TestParameter boolean keepGoing) throws Exception { reporter.removeHandler(failFastHandler); - getSkyframeExecutor().turnOffSyscallCacheForTesting(); scratch.file("parent/BUILD", "sh_library(name = 'parent')").getParentDirectory(); AtomicBoolean returnNull = new AtomicBoolean(false); this.transformer = @@ -162,7 +160,6 @@ public FileStatus stat(FileStatus stat, PathFragment path, boolean followSymlink @Test public void testBadReaddirKeepGoing() throws Exception { reporter.removeHandler(failFastHandler); - skyframeExecutor.turnOffSyscallCacheForTesting(); // Given a package, "parent", Path parent = scratch.file("parent/BUILD", "sh_library(name = 'parent')").getParentDirectory(); // And a child, "badstat", diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java index 73fb8eeb7c862c..04d614d06c3bbe 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java @@ -17,13 +17,11 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import static com.google.devtools.build.lib.testutil.TestConstants.PLATFORM_LABEL; -import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.assertThrows; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.BuildOptionsView; @@ -47,24 +45,20 @@ import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery.Code; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.PathFragment; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; -import com.google.devtools.build.skyframe.NotifyingHelper; -import com.google.devtools.build.skyframe.TrackingAwaiter; import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Set; -import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; import org.junit.After; import org.junit.Before; @@ -551,17 +545,20 @@ public void testMultipleTopLevelConfigurations_multipleConfigsPrefersTopLevel() @Test public void inconsistentSkyQueryIncremental() throws Exception { PathFragment barBuild = PathFragment.create("bar/BUILD"); - PathFragment bar = barBuild.getParentDirectory(); PostAnalysisQueryHelper.AnalysisHelper analysisHelper = new PostAnalysisQueryHelper.AnalysisHelper() { @Override protected FileSystem createFileSystem() { return new InMemoryFileSystem(BlazeClock.instance(), DigestHashFunction.SHA256) { + private final AtomicInteger barStatCount = new AtomicInteger(0); + @Nullable @Override public FileStatus statIfFound(PathFragment path, boolean followSymlinks) throws IOException { - return path.endsWith(barBuild) ? null : super.statIfFound(path, followSymlinks); + return path.endsWith(barBuild) && barStatCount.incrementAndGet() == 2 + ? null + : super.statIfFound(path, followSymlinks); } }; } @@ -573,31 +570,8 @@ protected FlagBuilder defaultFlags() { return super.defaultFlags().with(Flag.KEEP_GOING); } }; - CountDownLatch directoryListingLatch = new CountDownLatch(1); createQueryHelper(); getHelper().setUp(analysisHelper); - // Make sure the directory listing populates the syscalls cache before the stat is done, so that - // the stat can happen after syscalls cache has already confirmed that it's a file. The usual - // testing technique of turning off the syscalls cache is undone via the preliminaries that - // AnalysisTestCase performs on each evaluation. - analysisHelper - .getSkyframeExecutor() - .getEvaluator() - .injectGraphTransformerForTesting( - NotifyingHelper.makeNotifyingTransformer( - (key, type, order, context) -> { - if (NotifyingHelper.EventType.IS_READY.equals(type) - && FileStateValue.FILE_STATE.equals(key.functionName()) - && barBuild.equals(((RootedPath) key.argument()).getRootRelativePath())) { - TrackingAwaiter.INSTANCE.awaitLatchAndTrackExceptions( - directoryListingLatch, "Directory never listed"); - } else if (NotifyingHelper.EventType.SET_VALUE.equals(type) - && NotifyingHelper.Order.AFTER.equals(order) - && SkyFunctions.DIRECTORY_LISTING_STATE.equals(key.functionName()) - && bar.equals(((RootedPath) key.argument()).getRootRelativePath())) { - directoryListingLatch.countDown(); - } - })); disableOrderedResults(); writeFile("foo/BUILD"); writeFile("bar/BUILD"); @@ -615,8 +589,6 @@ protected FlagBuilder defaultFlags() { QueryException queryException = assertThrows(QueryException.class, () -> eval("bar")); assertThat(queryException.getFailureDetail().getTargetPatterns().getCode()) .isEqualTo(FailureDetails.TargetPatterns.Code.CANNOT_DETERMINE_TARGET_FROM_FILENAME); - TrackingAwaiter.INSTANCE.assertNoErrors(); - assertThat(directoryListingLatch.await(0, SECONDS)).isTrue(); } private void writeSimpleTarget() throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java index a66d40250e4e42..e9f5c4392ed8d7 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/SkyframeQueryHelper.java @@ -66,6 +66,7 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.skyframe.MemoizingEvaluator; import com.google.devtools.build.skyframe.SkyKey; @@ -331,6 +332,7 @@ protected SkyframeExecutor createSkyframeExecutor(ConfiguredRuleClassProvider ru .setIgnoredPackagePrefixesFunction( new IgnoredPackagePrefixesFunction(ignoredPackagePrefixesFile)) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .build(); skyframeExecutor.injectExtraPrecomputedValues( ImmutableList.of( diff --git a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java index 097553d626ea48..a07ec046891807 100644 --- a/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java +++ b/src/test/java/com/google/devtools/build/lib/repository/ExternalPackageHelperTest.java @@ -142,7 +142,7 @@ public void createEnvironment() { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index d520b55f549112..28ee40a863d775 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -181,7 +181,7 @@ public void setupDelegator() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put(FileValue.FILE, new FileFunction(pkgLocator)) .put(SkyFunctions.REPOSITORY_DIRECTORY, delegatorFunction) diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java index 3639fe9c2c9828..799d156d8a24b2 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/BlazeRuntimeTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParser; @@ -103,6 +104,7 @@ public void crashTest() throws Exception { commandThread, VersionCommand.class.getAnnotation(Command.class), options, + SyscallCache.NO_CACHE, ImmutableList.of(), 0L, 0L, @@ -151,6 +153,7 @@ public void resultExtensions() throws Exception { Thread.currentThread(), VersionCommand.class.getAnnotation(Command.class), OptionsParser.builder().optionsClasses(COMMAND_ENV_REQUIRED_OPTIONS).build(), + SyscallCache.NO_CACHE, ImmutableList.of(), 0L, 0L, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java index c5b420f9b0a3b2..9f592a59f909bc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.build.skyframe.EvaluationContext; import com.google.devtools.build.skyframe.EvaluationResult; @@ -293,6 +294,7 @@ private void initEvaluator() throws AbruptExitException, InterruptedException, I /*workspaceStatusActionFactory=*/ null, /*diffAwarenessFactories=*/ ImmutableList.of(), getExtraSkyFunctions(), + SyscallCache.NO_CACHE, /*repositoryHelpersHolder=*/ null, SkyframeExecutor.SkyKeyStateReceiver.NULL_INSTANCE, BugReporter.defaultInstance()); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java index 743a1f6c566613..ac3e591760c3a0 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java @@ -105,13 +105,13 @@ public void baseSetUp() throws Exception { new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put(FileValue.FILE, new FileFunction(pkgLocator)) .put( Artifact.ARTIFACT, new ArtifactFunction( - () -> true, MetadataConsumerForMetrics.NO_OP, () -> SyscallCache.NO_CACHE)) + () -> true, MetadataConsumerForMetrics.NO_OP, SyscallCache.NO_CACHE)) .put(SkyFunctions.ACTION_EXECUTION, new SimpleActionExecutionFunction()) .put( SkyFunctions.PACKAGE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java index 88b4127a8da0db..99e4e397251061 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java @@ -128,13 +128,13 @@ public final void setUp() throws Exception { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); skyFunctions.put( WorkspaceFileValue.WORKSPACE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java index f73cace1925e9e..ced01a7e76ca38 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java @@ -170,7 +170,7 @@ private MemoizingEvaluator makeEvaluator(ExternalFileAction externalFileAction) new FileStateFunction( Suppliers.ofInstance( new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)) .put( FileSymlinkCycleUniquenessFunction.NAME, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java index 4c65a4284aa0bc..60df6c60634134 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java @@ -106,16 +106,16 @@ public void setUp() throws Exception { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); skyFunctions.put( SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, - new RecursiveFilesystemTraversalFunction(() -> SyscallCache.NO_CACHE)); + new RecursiveFilesystemTraversalFunction(SyscallCache.NO_CACHE)); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index 788c5477890224..c0155de71bd070 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -285,7 +285,7 @@ public void setUp() throws Exception { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java index d98f70d35b69ce..0345f5f724f5be 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java @@ -162,7 +162,7 @@ private Map createFunctionMap() { skyFunctions.put(SkyFunctions.GLOB, new GlobFunction(alwaysUseDirListing())); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, @@ -178,7 +178,7 @@ private Map createFunctionMap() { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java index 33595ef41f8bc6..13a7e946d35d2d 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java @@ -98,13 +98,13 @@ public final void setUp() throws Exception { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider(); skyFunctions.put( WorkspaceFileValue.WORKSPACE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java index d7c0550e399dbd..5c6edf86f556d3 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java @@ -357,7 +357,6 @@ public long getNodeId() throws IOException { @Test public void testPropagatesFilesystemInconsistencies_globbing() throws Exception { - getSkyframeExecutor().turnOffSyscallCacheForTesting(); RecordingDifferencer differencer = getSkyframeExecutor().getDifferencerForTesting(); Root pkgRoot = getSkyframeExecutor().getPathEntries().get(0); scratch.file( @@ -392,11 +391,6 @@ public void testPropagatesFilesystemInconsistencies_globbing() throws Exception /** Regression test for unexpected exception type from PackageValue. */ @Test public void testDiscrepancyBetweenGlobbingErrors() throws Exception { - // Normally, non-Skyframe globbing and skyframe globbing share a cache for `readdir` filesystem - // calls. In order to exercise a situation where they observe different results for filesystem - // calls, we disable the cache. This might happen in a real scenario, e.g. if the cache hits a - // limit and evicts entries. - getSkyframeExecutor().turnOffSyscallCacheForTesting(); Path fooBuildFile = scratch.file("foo/BUILD", "sh_library(name = 'foo', srcs = glob(['bar/*.sh']))"); Path fooDir = fooBuildFile.getParentDirectory(); @@ -1320,7 +1314,6 @@ public void veryBrokenPackagePostsDoneToProgressReceiver() throws Exception { @Test public void testNonSkyframeGlobbingEncountersSymlinkCycleAndThrowsIOException() throws Exception { reporter.removeHandler(failFastHandler); - getSkyframeExecutor().turnOffSyscallCacheForTesting(); // When a package's BUILD file and the relevant filesystem state is such that non-Skyframe // globbing will encounter an IOException due to a directory symlink cycle, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index b65d84ed51cdb8..ba2df2ec1d2be5 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -133,13 +133,13 @@ public final void setUp() throws Exception { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); skyFunctions.put( SkyFunctions.IGNORED_PACKAGE_PREFIXES, new IgnoredPackagePrefixesFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PathCasingLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PathCasingLookupFunctionTest.java index 56ded76f687073..1096d4f5e4633f 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PathCasingLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PathCasingLookupFunctionTest.java @@ -84,13 +84,13 @@ public void setUp() { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); skyFunctions.put(SkyFunctions.PATH_CASING_LOOKUP, new PathCasingLookupFunction()); differencer = new SequencedRecordingDifferencer(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java index 798b6f8a1a94a6..1feedf4144c2f8 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.EvaluationContext; import com.google.devtools.build.skyframe.EvaluationResult; import com.google.devtools.build.skyframe.SkyKey; @@ -90,6 +91,7 @@ public void setUp() throws Exception { .setDirectories(directories) .setActionKeyContext(new ActionKeyContext()) .setExtraSkyFunctions(AnalysisMock.get().getSkyFunctions(directories)) + .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .setIgnoredPackagePrefixesFunction( new IgnoredPackagePrefixesFunction( PathFragment.create(ADDITIONAL_IGNORED_PACKAGE_PREFIXES_FILE_PATH_STRING))) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index b568858c390c06..2526d5bcac7e6e 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java @@ -146,16 +146,16 @@ public void setUp() { FileStateValue.FILE_STATE, new FileStateFunction( Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, externalFilesHelper)); skyFunctions.put(FileValue.FILE, new FileFunction(pkgLocator)); skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.DIRECTORY_LISTING_STATE, - new DirectoryListingStateFunction(externalFilesHelper, () -> SyscallCache.NO_CACHE)); + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)); skyFunctions.put( SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, - new RecursiveFilesystemTraversalFunction(() -> SyscallCache.NO_CACHE)); + new RecursiveFilesystemTraversalFunction(SyscallCache.NO_CACHE)); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 2efd420bfc8a0b..595b758b2dfe6a 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java @@ -237,7 +237,7 @@ protected BuilderWithResult createBuilder( MetadataConsumerForMetrics.NO_OP, new AtomicReference<>(statusReporter), /*sourceRootSupplier=*/ ImmutableList::of, - () -> SyscallCache.NO_CACHE, + SyscallCache.NO_CACHE, k -> ThreadStateReceiver.NULL_INSTANCE); Path actionOutputBase = scratch.dir("/usr/local/google/_blaze_jrluser/FAKEMD5/action_out/"); @@ -255,13 +255,12 @@ protected BuilderWithResult createBuilder( ImmutableMap.builder() .put( FileStateValue.FILE_STATE, - new FileStateFunction( - () -> tsgm, () -> SyscallCache.NO_CACHE, externalFilesHelper)) + new FileStateFunction(() -> tsgm, SyscallCache.NO_CACHE, externalFilesHelper)) .put(FileValue.FILE, new FileFunction(pkgLocator)) .put( Artifact.ARTIFACT, new ArtifactFunction( - () -> true, MetadataConsumerForMetrics.NO_OP, () -> SyscallCache.NO_CACHE)) + () -> true, MetadataConsumerForMetrics.NO_OP, SyscallCache.NO_CACHE)) .put( SkyFunctions.ACTION_EXECUTION, new ActionExecutionFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java index 0cd0436bdf37b4..217fbddafb1c9d 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java @@ -475,12 +475,6 @@ public void testListBindFunction() throws Exception { @Test public void testWorkspaceFileValueListener() throws Exception { - // Normally, syscalls cache is reset in the sync() method of the SkyframeExecutor, before - // diffing. - // But here we are calling only actual diffing part, exposed for testing: - // handleDiffsForTesting(), so we better turn off the syscalls cache. - skyframeExecutor.turnOffSyscallCacheForTesting(); - createWorkspaceFile("workspace(name = 'old')"); skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE); assertThat(testManagedDirectoriesKnowledge.getLastWorkspaceName()).isEqualTo("old"); diff --git a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java index 05ca3f755144a8..78f8def01458e5 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java @@ -274,7 +274,7 @@ private Set resolvePaths(String... relativePaths) { } @Test - public void testIOFailureOnStat() throws Exception { + public void testIOFailureOnStat() { SyscallCache syscalls = new SyscallCache() { @Override @@ -291,6 +291,11 @@ public Collection readdir(Path path) { public Dirent.Type getType(Path path, Symlinks symlinks) { throw new IllegalStateException(); } + + @Override + public void clear() { + throw new IllegalStateException(); + } }; IOException e = @@ -322,6 +327,11 @@ public Collection readdir(Path path) { public Dirent.Type getType(Path path, Symlinks symlinks) { throw new IllegalStateException(); } + + @Override + public void clear() { + throw new IllegalStateException(); + } }; assertThat(