From de7b26acfdfcd9c36ec957305a889ac29f0da30e Mon Sep 17 00:00:00 2001
From: Googler <jhorvitz@google.com>
Date: Fri, 2 Dec 2022 14:26:11 -0800
Subject: [PATCH] Restrict syscall caching to paths backed by the build's main
 `FileSystem`.

Avoids wasteful caching of in-memory and transient `FileSystem` types.

PiperOrigin-RevId: 492560997
Change-Id: I19865c33f91566fb101c98508fe8897f9ad115f5
---
 .../build/lib/runtime/WorkspaceBuilder.java   |  11 +-
 .../lib/vfs/SingleFileSystemSyscallCache.java | 101 ++++++++++++++++++
 2 files changed, 109 insertions(+), 3 deletions(-)
 create mode 100644 src/main/java/com/google/devtools/build/lib/vfs/SingleFileSystemSyscallCache.java

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 e1a6244b4b01f0..f4fcabc377d8aa 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
@@ -30,6 +30,7 @@
 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.SingleFileSystemSyscallCache;
 import com.google.devtools.build.lib.vfs.SyscallCache;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionName;
@@ -96,7 +97,8 @@ public static PerBuildSyscallCache createPerBuildSyscallCache() {
   BlazeWorkspace build(
       BlazeRuntime runtime,
       PackageFactory packageFactory,
-      SubscriberExceptionHandler eventBusExceptionHandler) throws AbruptExitException {
+      SubscriberExceptionHandler eventBusExceptionHandler)
+      throws AbruptExitException {
     // Set default values if none are set.
     if (skyframeExecutorFactory == null) {
       skyframeExecutorFactory = new SequencedSkyframeExecutorFactory();
@@ -105,6 +107,9 @@ BlazeWorkspace build(
       perCommandSyscallCache = createPerBuildSyscallCache();
     }
 
+    SingleFileSystemSyscallCache singleFsSyscallCache =
+        new SingleFileSystemSyscallCache(perCommandSyscallCache, runtime.getFileSystem());
+
     SkyframeExecutor skyframeExecutor =
         skyframeExecutorFactory.create(
             packageFactory,
@@ -114,7 +119,7 @@ BlazeWorkspace build(
             workspaceStatusActionFactory,
             diffAwarenessFactories.build(),
             skyFunctions.buildOrThrow(),
-            perCommandSyscallCache,
+            singleFsSyscallCache,
             skyframeExecutorRepositoryHelpersHolder,
             skyKeyStateReceiver == null
                 ? SkyframeExecutor.SkyKeyStateReceiver.NULL_INSTANCE
@@ -128,7 +133,7 @@ BlazeWorkspace build(
         workspaceStatusActionFactory,
         binTools,
         allocationTracker,
-        perCommandSyscallCache);
+        singleFsSyscallCache);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/SingleFileSystemSyscallCache.java b/src/main/java/com/google/devtools/build/lib/vfs/SingleFileSystemSyscallCache.java
new file mode 100644
index 00000000000000..9befa7f1e9741f
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/vfs/SingleFileSystemSyscallCache.java
@@ -0,0 +1,101 @@
+// Copyright 2022 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.vfs;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.io.IOException;
+import java.util.Collection;
+import javax.annotation.Nullable;
+
+/**
+ * A {@link SyscallCache} that delegates to a caching implementation only for paths with a
+ * particular {@link FileSystem}.
+ *
+ * <p>Any calls that pass a {@link Path} backed by a different {@link FileSystem} are routed to
+ * {@link SyscallCache#NO_CACHE}. This can be used to ensure that only calls for the build's main
+ * {@link FileSystem} are cached. Common alternative filesystems for which caching is wasteful
+ * include:
+ *
+ * <ul>
+ *   <li>{@link
+ *       com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.BundledFileSystem}, an
+ *       in-memory filesystem (no real filesystem ops to save).
+ *   <li>An {@linkplain com.google.devtools.build.lib.vfs.OutputService.ActionFileSystemType
+ *       action-scoped filesystem} where there is no potential for reuse outside a single action's
+ *       execution, and caching prolongs the lifetime of the instance.
+ */
+public final class SingleFileSystemSyscallCache implements SyscallCache {
+
+  private final SyscallCache delegate;
+  private final FileSystem fs;
+
+  public SingleFileSystemSyscallCache(SyscallCache delegate, FileSystem fs) {
+    this.delegate = checkNotNull(delegate);
+    this.fs = checkNotNull(fs);
+  }
+
+  @Override
+  public Collection<Dirent> readdir(Path path) throws IOException {
+    return delegateFor(path).readdir(path);
+  }
+
+  @Nullable
+  @Override
+  public FileStatus statIfFound(Path path, Symlinks symlinks) throws IOException {
+    return delegateFor(path).statIfFound(path, symlinks);
+  }
+
+  @Nullable
+  @Override
+  public DirentTypeWithSkip getType(Path path, Symlinks symlinks) throws IOException {
+    return delegateFor(path).getType(path, symlinks);
+  }
+
+  @Override
+  public byte[] getFastDigest(Path path) throws IOException {
+    return delegateFor(path).getFastDigest(path);
+  }
+
+  @Override
+  public byte[] getxattr(Path path, String xattrName) throws IOException {
+    return delegateFor(path).getxattr(path, xattrName);
+  }
+
+  @Override
+  public byte[] getxattr(Path path, String xattrName, Symlinks followSymlinks) throws IOException {
+    return delegateFor(path).getxattr(path, xattrName, followSymlinks);
+  }
+
+  @Override
+  public void noteAnalysisPhaseEnded() {
+    delegate.noteAnalysisPhaseEnded();
+  }
+
+  @Override
+  public void clear() {
+    delegate.clear();
+  }
+
+  /** Returns the underlying {@link SyscallCache} used for eligible paths. */
+  // TODO(b/245929310): This shouldn't be exposed.
+  public SyscallCache getUnderlyingCache() {
+    return delegate;
+  }
+
+  private SyscallCache delegateFor(Path path) {
+    return path.getFileSystem().equals(fs) ? delegate : SyscallCache.NO_CACHE;
+  }
+}