commit a0a72265ea37270c110630f528127e18939f4016
author Alexander Golovlev <golovlev@gmail.com> Tue Oct 29 04:26:22 2024 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Oct 29 04:27:59 2024 -0700
tree 3d137d748163ab9f7a10f3977c1746474b042a4d
parent ba86658080fd8940334e66a5426ecbf5c630f322

Performance optimization for isSymbolicLink() calls on Windows

Current implementation for WindowsFileSystem.isSymbolicLink() makes unnecessary system calls which significantly affect the performance. Original code goes through the following:
- AbstractFileSystemWithCustomStat.isSymbolicLink
  - WindowsFileSystem.stat
    - WindowsFileSystem.getIoFile
    - JavaIoFileSystem.getNioPath
    - Files.readAttributes
    - WindowsFileSystem.fileIsSymbolicLink
    - WindowsFileOperations.getLastChangeTime
This implementation skips most of them.

Closes #24047.

PiperOrigin-RevId: 690964117
Change-Id: I2b407d9c69af62e770684d868d04e60d7ce1773e

src/main/java/com/google/devtools/build/lib/windows/BUILD

diff --git a/src/main/java/com/google/devtools/build/lib/windows/BUILD b/src/main/java/com/google/devtools/build/lib/windows/BUILD
index 20809bb..8294631 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/windows/BUILD
@@ -42,5 +42,6 @@
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//third_party:guava",
+        "//third_party:jsr305",
     ],
 )

src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java

diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index 9c35178..78b4c23 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -27,6 +27,7 @@
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.attribute.DosFileAttributes;
+import javax.annotation.Nullable;
 
 /** File system implementation for Windows. */
 @ThreadSafe
@@ -148,14 +149,14 @@
       throw new FileNotFoundException(path + ERR_NO_SUCH_FILE_OR_DIR);
     }
 
-    final boolean isSymbolicLink = !followSymlinks && fileIsSymbolicLink(file);
-    final long lastChangeTime =
-        WindowsFileOperations.getLastChangeTime(getNioPath(path).toString(), followSymlinks);
     FileStatus status =
         new FileStatus() {
+          @Nullable volatile Boolean isSymbolicLink; // null if not yet known
+          volatile long lastChangeTime = -1;
+
           @Override
           public boolean isFile() {
-            return !isSymbolicLink && (attributes.isRegularFile() || isSpecialFile());
+            return !isSymbolicLink() && (attributes.isRegularFile() || isSpecialFile());
           }
 
           @Override
@@ -163,16 +164,19 @@
             // attributes.isOther() returns false for symlinks but returns true for junctions.
             // Bazel treats junctions like symlinks. So let's return false here for junctions.
             // This fixes https://github.com/bazelbuild/bazel/issues/9176
-            return !isSymbolicLink && attributes.isOther();
+            return !isSymbolicLink() && attributes.isOther();
           }
 
           @Override
           public boolean isDirectory() {
-            return !isSymbolicLink && attributes.isDirectory();
+            return !isSymbolicLink() && attributes.isDirectory();
           }
 
           @Override
           public boolean isSymbolicLink() {
+            if (isSymbolicLink == null) {
+              isSymbolicLink = !followSymlinks && fileIsSymbolicLink(file);
+            }
             return isSymbolicLink;
           }
 
@@ -187,7 +191,12 @@
           }
 
           @Override
-          public long getLastChangeTime() {
+          public long getLastChangeTime() throws IOException {
+            if (lastChangeTime == -1) {
+              lastChangeTime =
+                  WindowsFileOperations.getLastChangeTime(
+                      getNioPath(path).toString(), followSymlinks);
+            }
             return lastChangeTime;
           }
 
@@ -208,6 +217,11 @@
   }
 
   @Override
+  protected boolean isSymbolicLink(PathFragment path) {
+    return fileIsSymbolicLink(getIoFile(path));
+  }
+
+  @Override
   protected boolean isDirectory(PathFragment path, boolean followSymlinks) {
     if (!followSymlinks) {
       try {