commit c5377c17c191c85c608b8cafbf48d1d1764f5e48
author Googler <jhorvitz@google.com> Wed Mar 15 10:00:58 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Mar 15 10:02:02 2023 -0700
tree 2d73112aae9b9af2d9d6c2e8e92c2d4cca1b3e85
parent d3005474aa2071308a45a7d285f6eeaf30a4a9e1

Intern `RootedPath`.

The recently added `SkyKeyInterner` feature makes this much more worthwhile.

To offset the extra cost of creating a `RootedPath` instance, try to avoid it if possible.

Delete unused `RootedPathAndCasing` which relied on `RootedPath` not being interned on case-insensitive filesystems.

PiperOrigin-RevId: 516855266
Change-Id: I1d5365abe59b11e77305d247f24ff9a6677f30e9

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
index 76414eb..0ba6700 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java
@@ -78,10 +78,10 @@
     // In the course of resolving the real path of p, there will be a logical chain of paths we
     // consider. Going with the example from above, the full chain of paths we consider is
     // [a/b, c/b].
-    ArrayList<RootedPath> logicalChain = new ArrayList<>();
-    // Same contents as 'logicalChain', except stored as an sorted TreeSet for efficiency reasons.
+    final ArrayList<RootedPath> logicalChain = new ArrayList<>();
+    // Same contents as 'logicalChain', except stored as a sorted TreeSet for efficiency reasons.
     // See the usage in checkPathSeenDuringPartialResolutionInternal.
-    TreeSet<Path> sortedLogicalChain = Sets.newTreeSet();
+    final TreeSet<Path> sortedLogicalChain = Sets.newTreeSet();
 
     ImmutableList<RootedPath> pathToUnboundedAncestorSymlinkExpansionChain = null;
     ImmutableList<RootedPath> unboundedAncestorSymlinkExpansionChain = null;
@@ -145,9 +145,13 @@
         realFileStateValue);
   }
 
-  private static RootedPath getChild(RootedPath parentRootedPath, String baseName) {
+  private static RootedPath getChild(
+      RootedPath parent, String baseName, RootedPath originalParent, RootedPath originalChild) {
+    if (parent.equals(originalParent)) {
+      return originalChild; // Avoid constructing a new instance if we already have the child.
+    }
     return RootedPath.toRootedPath(
-        parentRootedPath.getRoot(), parentRootedPath.getRootRelativePath().getChild(baseName));
+        parent.getRoot(), parent.getRootRelativePath().getChild(baseName));
   }
 
   private RootedPath toRootedPath(Path path) {
@@ -180,14 +184,15 @@
       Environment env)
       throws InterruptedException, FileFunctionException {
     PathFragment relativePath = rootedPath.getRootRelativePath();
-    RootedPath rootedPathFromAncestors;
     String baseName = relativePath.getBaseName();
 
     FileValue parentFileValue = (FileValue) env.getValue(FileValue.key(parentRootedPath));
     if (parentFileValue == null) {
       return null;
     }
-    rootedPathFromAncestors = getChild(parentFileValue.realRootedPath(), baseName);
+
+    RootedPath rootedPathFromAncestors =
+        getChild(parentFileValue.realRootedPath(), baseName, parentRootedPath, rootedPath);
 
     if (!parentFileValue.exists() || !parentFileValue.isDirectory()) {
       return new PartialResolutionResult(
@@ -196,7 +201,9 @@
 
     for (RootedPath parentPartialRootedPath : parentFileValue.logicalChainDuringResolution()) {
       checkAndNotePathSeenDuringPartialResolution(
-          getChild(parentPartialRootedPath, baseName), symlinkResolutionState, env);
+          getChild(parentPartialRootedPath, baseName, parentRootedPath, rootedPath),
+          symlinkResolutionState,
+          env);
       if (env.valuesMissing()) {
         return null;
       }