Improve the FileSystem inheritance hierarchy.

By repurposing AbstractFileSystem as DiskBackedFileSystem, to be extended only by disk-backed subclasses. This encourages other subclasses to make an explicit decision on how to open files instead of inheriting a potentially incorrect implementation.

As a bonus, we can also dispense with the duplicate profiling logic in UnixFileSystem.

PiperOrigin-RevId: 837473162
Change-Id: I970d0a9711ecc583b1ecae650703d14d4a5db2a4

Author: tjgq

src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java

@@ -40,7 +40,6 @@
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.remote.common.BulkTransferException;
 import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
-import com.google.devtools.build.lib.vfs.AbstractFileSystem;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
 import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.FileStatus;
@@ -101,8 +100,7 @@
  * sources, such as the same path existing in multiple underlying sources with different type or
  * contents.
  */
-public class RemoteActionFileSystem extends AbstractFileSystem
-    implements PathCanonicalizer.Resolver {
+public class RemoteActionFileSystem extends FileSystem implements PathCanonicalizer.Resolver {
   private final PathFragment execRoot;
   private final PathFragment outputBase;
   private final InputMetadataProvider inputArtifactData;
@@ -901,7 +899,7 @@ private Dirent maybeFollowSymlinkForDirent(
   @Override
   public void createFSDependentHardLink(PathFragment linkPath, PathFragment originalPath)
       throws IOException {
-    // Only called by the AbstractFileSystem#createHardLink base implementation, overridden below.
+    // Only called by the FileSystem#createHardLink base implementation, overridden below.
     throw new UnsupportedOperationException();
   }
 

src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java

@@ -21,28 +21,26 @@
 import com.google.devtools.build.lib.util.Blocker;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.util.StringEncoding;
-import com.google.devtools.build.lib.vfs.AbstractFileSystem;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
 import com.google.devtools.build.lib.vfs.Dirent;
+import com.google.devtools.build.lib.vfs.DiskBackedFileSystem;
 import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.FileSymlinkLoopException;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.SymlinkTargetType;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
 import java.util.ArrayDeque;
 import java.util.Collection;
 import javax.annotation.Nullable;
 
-/** This class implements the FileSystem interface using direct calls to the UNIX filesystem. */
+/**
+ * A disk-backed filesystem suitable for Unix systems, implemented using a mix of JNI and standard
+ * library calls.
+ */
 @ThreadSafe
-public class UnixFileSystem extends AbstractFileSystem {
+public class UnixFileSystem extends DiskBackedFileSystem {
   protected final String hashAttributeName;
 
   public UnixFileSystem(DigestHashFunction hashFunction, String hashAttributeName) {
@@ -441,80 +439,4 @@ public File getIoFile(PathFragment path) {
   public java.nio.file.Path getNioPath(PathFragment path) {
     return java.nio.file.Path.of(StringEncoding.internalToPlatform(path.getPathString()));
   }
-
-  @Override
-  protected InputStream createFileInputStream(PathFragment path) throws IOException {
-    return new FileInputStream(StringEncoding.internalToPlatform(path.getPathString()));
-  }
-
-  protected OutputStream createFileOutputStream(PathFragment path, boolean append)
-      throws FileNotFoundException {
-    return createFileOutputStream(path, append, /* internal= */ false);
-  }
-
-  @Override
-  protected OutputStream createFileOutputStream(PathFragment path, boolean append, boolean internal)
-      throws FileNotFoundException {
-    String name = path.getPathString();
-    var profiler = Profiler.instance();
-    if (!internal
-        && profiler.isActive()
-        && (profiler.isProfiling(ProfilerTask.VFS_WRITE)
-            || profiler.isProfiling(ProfilerTask.VFS_OPEN))) {
-      long startTime = Profiler.instance().nanoTimeMaybe();
-      var comp = Blocker.begin();
-      try {
-        return new ProfiledFileOutputStream(name, /* append= */ append);
-      } finally {
-        Blocker.end(comp);
-        profiler.logSimpleTask(startTime, ProfilerTask.VFS_OPEN, name);
-      }
-    } else {
-      var comp = Blocker.begin();
-      try {
-        return new FileOutputStream(StringEncoding.internalToPlatform(name), /* append= */ append);
-      } finally {
-        Blocker.end(comp);
-      }
-    }
-  }
-
-  private static final class ProfiledFileOutputStream extends FileOutputStream {
-    private final String name;
-
-    private ProfiledFileOutputStream(String name, boolean append) throws FileNotFoundException {
-      super(StringEncoding.internalToPlatform(name), append);
-      this.name = name;
-    }
-
-    @Override
-    public void write(int b) throws IOException {
-      long startTime = Profiler.instance().nanoTimeMaybe();
-      try {
-        super.write(b);
-      } finally {
-        Profiler.instance().logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name);
-      }
-    }
-
-    @Override
-    public void write(byte[] b) throws IOException {
-      long startTime = Profiler.instance().nanoTimeMaybe();
-      try {
-        super.write(b);
-      } finally {
-        Profiler.instance().logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name);
-      }
-    }
-
-    @Override
-    public void write(byte[] b, int off, int len) throws IOException {
-      long startTime = Profiler.instance().nanoTimeMaybe();
-      try {
-        super.write(b, off, len);
-      } finally {
-        Profiler.instance().logSimpleTask(startTime, ProfilerTask.VFS_WRITE, name);
-      }
-    }
-  }
 }