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 491e441: this changes many of the same lines.

PiperOrigin-RevId: 426688249

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",

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<? extends SyscallCache> 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<? extends SyscallCache> 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));

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,

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<String> 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 =

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

src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java

@@ -69,7 +69,7 @@
 class ArtifactFunction implements SkyFunction {
   private final Supplier<Boolean> mkdirForTreeArtifacts;
   private final MetadataConsumerForMetrics sourceArtifactsSeen;
-  private final Supplier<SyscallCache> 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<Boolean> mkdirForTreeArtifacts,
       MetadataConsumerForMetrics sourceArtifactsSeen,
-      Supplier<SyscallCache> 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);

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> syscallCache;
+  private final SyscallCache syscallCache;
 
   public DirectoryListingStateFunction(
-      ExternalFilesHelper externalFilesHelper, Supplier<SyscallCache> 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.

src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java

@@ -34,12 +34,12 @@
 public class FileStateFunction implements SkyFunction {
 
   private final Supplier<TimestampGranularityMonitor> tsgm;
-  private final Supplier<SyscallCache> syscallCache;
+  private final SyscallCache syscallCache;
   private final ExternalFilesHelper externalFilesHelper;
 
   public FileStateFunction(
       Supplier<TimestampGranularityMonitor> tsgm,
-      Supplier<SyscallCache> 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) {

src/main/java/com/google/devtools/build/lib/skyframe/PerBuildSyscallCache.java

@@ -29,7 +29,9 @@
 /**
  * A per-build cache of filesystem operations.
  *
- * <p>Mostly used by non-Skyframe globbing and include parsing.
+ * <p>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();

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> syscallCache;
+  private final SyscallCache syscallCache;
 
-  RecursiveFilesystemTraversalFunction(Supplier<SyscallCache> 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;
       }