Replace path implementation.

Path and PathFragment have been replaced with String-based implementations. They are pretty similar, but each method is dissimilar enough that I did not feel sharing code was appropriate.

A summary of changes:

PATH
====

* Subsumes LocalPath (deleted, its tests repurposed)
* Use a simple string to back Path
* Path instances are no longer interned; Reference equality will no longer work
* Always normalized (same as before)
* Some operations will now be slower, like instance compares (which were previously just a reference check)
* Multiple identical paths will now consume more memory since they are not interned

PATH FRAGMENT
=============

* Use a simple string to back PathFragment
* No more segment arrays with interned strings
* Always normalized
* Remove isNormalized
* Replace some isNormalizied uses with containsUpLevelReferences() to check if path fragments try to escape their scope
* To check if user input is normalized, supply static methods on PathFragment to validate the string before constructing a PathFragment
* Because PathFragments are always normalized, we have to replace checks for literal "." from PathFragment#getPathString to PathFragment#getSafePathString. The latter returns "." for the empty string.
* The previous implementation supported efficient segment semantics (segment count, iterating over segments). This is now expensive since we do longer have a segment array.

ARTIFACT
========

* Remove Path instance. It is instead dynamically constructed on request. This is necessary to avoid this CL becoming a memory regression.

RELNOTES: None
PiperOrigin-RevId: 185062932

src/main/java/com/google/devtools/build/lib/actions/Actions.java

@@ -23,7 +23,9 @@
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
+import com.google.devtools.build.lib.vfs.OsPathPolicy;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -149,7 +151,7 @@ private static GeneratingActions maybeFilterSharedActionsAndThrowIfConflict(
    */
   public static Map<ActionAnalysisMetadata, ArtifactPrefixConflictException>
       findArtifactPrefixConflicts(Map<Artifact, ActionAnalysisMetadata> generatingActions) {
-    TreeMap<PathFragment, Artifact> artifactPathMap = new TreeMap();
+    TreeMap<PathFragment, Artifact> artifactPathMap = new TreeMap<>(comparatorForPrefixConflicts());
     for (Artifact artifact : generatingActions.keySet()) {
       artifactPathMap.put(artifact.getExecPath(), artifact);
     }
@@ -158,19 +160,64 @@ private static GeneratingActions maybeFilterSharedActionsAndThrowIfConflict(
         new MapBasedImmutableActionGraph(generatingActions), artifactPathMap);
   }
 
+  /**
+   * Returns a comparator for use with {@link #findArtifactPrefixConflicts(ActionGraph, SortedMap)}.
+   */
+  public static Comparator<PathFragment> comparatorForPrefixConflicts() {
+    return PathFragmentPrefixComparator.INSTANCE;
+  }
+
+  private static class PathFragmentPrefixComparator implements Comparator<PathFragment> {
+    private static final PathFragmentPrefixComparator INSTANCE = new PathFragmentPrefixComparator();
+
+    @Override
+    public int compare(PathFragment lhs, PathFragment rhs) {
+      // We need to use the OS path policy in case the OS is case insensitive.
+      OsPathPolicy os = OsPathPolicy.getFilePathOs();
+      String str1 = lhs.getPathString();
+      String str2 = rhs.getPathString();
+      int len1 = str1.length();
+      int len2 = str2.length();
+      int n = Math.min(len1, len2);
+      for (int i = 0; i < n; ++i) {
+        char c1 = str1.charAt(i);
+        char c2 = str2.charAt(i);
+        int res = os.compare(c1, c2);
+        if (res != 0) {
+          if (c1 == PathFragment.SEPARATOR_CHAR) {
+            return -1;
+          } else if (c2 == PathFragment.SEPARATOR_CHAR) {
+            return 1;
+          }
+          return res;
+        }
+      }
+      return len1 - len2;
+    }
+  }
+
   /**
    * Finds Artifact prefix conflicts between generated artifacts. An artifact prefix conflict
    * happens if one action generates an artifact whose path is a prefix of another artifact's path.
    * Those two artifacts cannot exist simultaneously in the output tree.
    *
    * @param actionGraph the {@link ActionGraph} to query for artifact conflicts
-   * @param artifactPathMap a map mapping generated artifacts to their exec paths
+   * @param artifactPathMap a map mapping generated artifacts to their exec paths. The map must be
+   *     sorted using the comparator from {@link #comparatorForPrefixConflicts()}.
    * @return A map between actions that generated the conflicting artifacts and their associated
    *     {@link ArtifactPrefixConflictException}.
    */
   public static Map<ActionAnalysisMetadata, ArtifactPrefixConflictException>
-      findArtifactPrefixConflicts(ActionGraph actionGraph,
-      SortedMap<PathFragment, Artifact> artifactPathMap) {
+      findArtifactPrefixConflicts(
+          ActionGraph actionGraph, SortedMap<PathFragment, Artifact> artifactPathMap) {
+    // You must construct the sorted map using this comparator for the algorithm to work.
+    // The algorithm requires subdirectories to immediately follow parent directories,
+    // before any files in that directory.
+    // Example: "foo", "foo.obj", foo/bar" must be sorted
+    // "foo", "foo/bar", foo.obj"
+    Preconditions.checkArgument(
+        artifactPathMap.comparator() instanceof PathFragmentPrefixComparator,
+        "artifactPathMap must be sorted with PathFragmentPrefixComparator");
     // No actions in graph -- currently happens only in tests. Special-cased because .next() call
     // below is unconditional.
     if (artifactPathMap.isEmpty()) {

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -176,7 +176,6 @@ public interface ArtifactExpander {
   public static final Predicate<Artifact> MIDDLEMAN_FILTER = input -> !input.isMiddlemanArtifact();
 
   private final int hashCode;
-  private final Path path;
   private final ArtifactRoot root;
   private final PathFragment execPath;
   private final PathFragment rootRelativePath;
@@ -203,35 +202,26 @@ public interface ArtifactExpander {
    */
   @VisibleForTesting
   @AutoCodec.Instantiator
-  public Artifact(Path path, ArtifactRoot root, PathFragment execPath, ArtifactOwner owner) {
-    if (root == null || !root.getRoot().contains(path)) {
-      throw new IllegalArgumentException(root + ": illegal root for " + path
-          + " (execPath: " + execPath + ")");
-    }
-    if (execPath == null
-        || execPath.isAbsolute() != root.getRoot().isAbsolute()
-        || !path.asFragment().endsWith(execPath)) {
-      throw new IllegalArgumentException(execPath + ": illegal execPath for " + path
-          + " (root: " + root + ")");
+  public Artifact(ArtifactRoot root, PathFragment execPath, ArtifactOwner owner) {
+    Preconditions.checkNotNull(root);
+    if (execPath == null || execPath.isAbsolute() != root.getRoot().isAbsolute()) {
+      throw new IllegalArgumentException(
+          execPath + ": illegal execPath for " + execPath + " (root: " + root + ")");
     }
     if (execPath.isEmpty()) {
       throw new IllegalArgumentException(
           "it is illegal to create an artifact with an empty execPath");
     }
-    this.hashCode = path.hashCode();
-    this.path = path;
+    this.hashCode = execPath.hashCode();
     this.root = root;
     this.execPath = execPath;
-    // These two lines establish the invariant that
-    // execPath == rootRelativePath <=> execPath.equals(rootRelativePath)
-    // This is important for isSourceArtifact.
-    PathFragment rootRel = root.getRoot().relativize(path);
-    if (!execPath.endsWith(rootRel)) {
-      throw new IllegalArgumentException(execPath + ": illegal execPath doesn't end with "
-          + rootRel + " at " + path + " with root " + root);
+    PathFragment rootExecPath = root.getExecPath();
+    if (rootExecPath.isEmpty()) {
+      this.rootRelativePath = execPath;
+    } else {
+      this.rootRelativePath = execPath.relativeTo(root.getExecPath());
     }
-    this.rootRelativePath = rootRel.equals(execPath) ? execPath : rootRel;
-    this.owner = Preconditions.checkNotNull(owner, path);
+    this.owner = Preconditions.checkNotNull(owner);
   }
 
   /**
@@ -251,8 +241,8 @@ public Artifact(Path path, ArtifactRoot root, PathFragment execPath, ArtifactOwn
    * <pre>
    */
   @VisibleForTesting
-  public Artifact(Path path, ArtifactRoot root, PathFragment execPath) {
-    this(path, root, execPath, ArtifactOwner.NullArtifactOwner.INSTANCE);
+  public Artifact(ArtifactRoot root, PathFragment execPath) {
+    this(root, execPath, ArtifactOwner.NullArtifactOwner.INSTANCE);
   }
 
   /**
@@ -262,7 +252,6 @@ public Artifact(Path path, ArtifactRoot root, PathFragment execPath) {
   @VisibleForTesting // Only exists for testing.
   public Artifact(Path path, ArtifactRoot root) {
     this(
-        path,
         root,
         root.getExecPath().getRelative(root.getRoot().relativize(path)),
         ArtifactOwner.NullArtifactOwner.INSTANCE);
@@ -272,14 +261,13 @@ public Artifact(Path path, ArtifactRoot root) {
   @VisibleForTesting // Only exists for testing.
   public Artifact(PathFragment rootRelativePath, ArtifactRoot root) {
     this(
-        root.getRoot().getRelative(rootRelativePath),
         root,
         root.getExecPath().getRelative(rootRelativePath),
         ArtifactOwner.NullArtifactOwner.INSTANCE);
   }
 
   public final Path getPath() {
-    return path;
+    return root.getRoot().getRelative(rootRelativePath);
   }
 
   public boolean hasParent() {
@@ -412,9 +400,8 @@ public PathFragment getParentRelativePath() {
     structField = true,
     doc = "Returns true if this is a source file, i.e. it is not generated."
   )
-  @SuppressWarnings("ReferenceEquality")  // == is an optimization
   public final boolean isSourceArtifact() {
-    return execPath == rootRelativePath;
+    return root.isSourceRoot();
   }
 
   /**
@@ -479,12 +466,8 @@ public static final class SpecialArtifact extends Artifact {
 
     @VisibleForSerialization
     public SpecialArtifact(
-        Path path,
-        ArtifactRoot root,
-        PathFragment execPath,
-        ArtifactOwner owner,
-        SpecialArtifactType type) {
-      super(path, root, execPath, owner);
+        ArtifactRoot root, PathFragment execPath, ArtifactOwner owner, SpecialArtifactType type) {
+      super(root, execPath, owner);
       this.type = type;
     }
 
@@ -559,17 +542,16 @@ public static final class TreeFileArtifact extends Artifact {
     TreeFileArtifact(
         SpecialArtifact parentTreeArtifact, PathFragment parentRelativePath, ArtifactOwner owner) {
       super(
-          parentTreeArtifact.getPath().getRelative(parentRelativePath),
           parentTreeArtifact.getRoot(),
           parentTreeArtifact.getExecPath().getRelative(parentRelativePath),
           owner);
-      Preconditions.checkState(
+      Preconditions.checkArgument(
           parentTreeArtifact.isTreeArtifact(),
           "The parent of TreeFileArtifact (parent-relative path: %s) is not a TreeArtifact: %s",
           parentRelativePath,
           parentTreeArtifact);
-      Preconditions.checkState(
-          parentRelativePath.isNormalized() && !parentRelativePath.isAbsolute(),
+      Preconditions.checkArgument(
+          !parentRelativePath.containsUplevelReferences() && !parentRelativePath.isAbsolute(),
           "%s is not a proper normalized relative path",
           parentRelativePath);
       this.parentTreeArtifact = parentTreeArtifact;
@@ -668,7 +650,7 @@ public final boolean equals(Object other) {
     // We don't bother to check root in the equivalence relation, because we
     // assume that no root is an ancestor of another one.
     Artifact that = (Artifact) other;
-    return Objects.equals(this.path, that.path);
+    return Objects.equals(this.execPath, that.execPath) && Objects.equals(this.root, that.root);
   }
 
   @Override
@@ -702,7 +684,7 @@ public final String toDetailString() {
       return "[" + root + "]" + rootRelativePath;
     } else {
       // Derived Artifact: path and root are under execRoot
-      PathFragment execRoot = trimTail(path.asFragment(), execPath);
+      PathFragment execRoot = trimTail(getPath().asFragment(), execPath);
       return "[["
           + execRoot
           + "]"

src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java

@@ -149,8 +149,7 @@ public Artifact getSourceArtifact(PathFragment execPath, ArtifactRoot root, Arti
     Preconditions.checkArgument(
         execPath.isAbsolute() == root.getRoot().isAbsolute(), "%s %s %s", execPath, root, owner);
     Preconditions.checkNotNull(owner, "%s %s", execPath, root);
-    execPath = execPath.normalize();
-    return getArtifact(root.getRoot().getRelative(execPath), root, execPath, owner, null);
+    return getArtifact(root, execPath, owner, null);
   }
 
   @Override
@@ -162,7 +161,7 @@ private void validatePath(PathFragment rootRelativePath, ArtifactRoot root) {
     Preconditions.checkArgument(!root.isSourceRoot());
     Preconditions.checkArgument(
         rootRelativePath.isAbsolute() == root.getRoot().isAbsolute(), rootRelativePath);
-    Preconditions.checkArgument(rootRelativePath.isNormalized(), rootRelativePath);
+    Preconditions.checkArgument(!rootRelativePath.containsUplevelReferences(), rootRelativePath);
     Preconditions.checkArgument(
         root.getRoot().asPath().startsWith(execRootParent), "%s %s", root, execRootParent);
     Preconditions.checkArgument(
@@ -182,8 +181,7 @@ private void validatePath(PathFragment rootRelativePath, ArtifactRoot root) {
   public Artifact getDerivedArtifact(
       PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
     validatePath(rootRelativePath, root);
-    Path path = root.getRoot().getRelative(rootRelativePath);
-    return getArtifact(path, root, root.getExecPath().getRelative(rootRelativePath), owner, null);
+    return getArtifact(root, root.getExecPath().getRelative(rootRelativePath), owner, null);
   }
 
   /**
@@ -197,13 +195,8 @@ public Artifact getDerivedArtifact(
   public Artifact getFilesetArtifact(
       PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
     validatePath(rootRelativePath, root);
-    Path path = root.getRoot().getRelative(rootRelativePath);
     return getArtifact(
-        path,
-        root,
-        root.getExecPath().getRelative(rootRelativePath),
-        owner,
-        SpecialArtifactType.FILESET);
+        root, root.getExecPath().getRelative(rootRelativePath), owner, SpecialArtifactType.FILESET);
   }
 
   /**
@@ -216,21 +209,14 @@ public Artifact getFilesetArtifact(
   public Artifact getTreeArtifact(
       PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
     validatePath(rootRelativePath, root);
-    Path path = root.getRoot().getRelative(rootRelativePath);
     return getArtifact(
-        path,
-        root,
-        root.getExecPath().getRelative(rootRelativePath),
-        owner,
-        SpecialArtifactType.TREE);
+        root, root.getExecPath().getRelative(rootRelativePath), owner, SpecialArtifactType.TREE);
   }
 
   public Artifact getConstantMetadataArtifact(
       PathFragment rootRelativePath, ArtifactRoot root, ArtifactOwner owner) {
     validatePath(rootRelativePath, root);
-    Path path = root.getRoot().getRelative(rootRelativePath);
     return getArtifact(
-        path,
         root,
         root.getExecPath().getRelative(rootRelativePath),
         owner,
@@ -242,7 +228,6 @@ public Artifact getConstantMetadataArtifact(
    * root</code> and <code>execPath</code> to the specified values.
    */
   private synchronized Artifact getArtifact(
-      Path path,
       ArtifactRoot root,
       PathFragment execPath,
       ArtifactOwner owner,
@@ -251,7 +236,7 @@ private synchronized Artifact getArtifact(
     Preconditions.checkNotNull(execPath);
 
     if (!root.isSourceRoot()) {
-      return createArtifact(path, root, execPath, owner, type);
+      return createArtifact(root, execPath, owner, type);
     }
 
     Artifact artifact = sourceArtifactCache.getArtifact(execPath);
@@ -260,23 +245,22 @@ private synchronized Artifact getArtifact(
       // There really should be a safety net that makes it impossible to create two Artifacts
       // with the same exec path but a different Owner, but we also need to reuse Artifacts from
       // previous builds.
-      artifact = createArtifact(path, root, execPath, owner, type);
+      artifact = createArtifact(root, execPath, owner, type);
       sourceArtifactCache.putArtifact(execPath, artifact);
     }
     return artifact;
   }
 
   private Artifact createArtifact(
-      Path path,
       ArtifactRoot root,
       PathFragment execPath,
       ArtifactOwner owner,
       @Nullable SpecialArtifactType type) {
-    Preconditions.checkNotNull(owner, path);
+    Preconditions.checkNotNull(owner);
     if (type == null) {
-      return new Artifact(path, root, execPath, owner);
+      return new Artifact(root, execPath, owner);
     } else {
-      return new Artifact.SpecialArtifact(path, root, execPath, owner, type);
+      return new Artifact.SpecialArtifact(root, execPath, owner, type);
     }
   }
 
@@ -301,7 +285,6 @@ public synchronized Artifact resolveSourceArtifactWithAncestor(
         !relativePath.isEmpty(), "%s %s %s", relativePath, baseExecPath, baseRoot);
     PathFragment execPath =
         baseExecPath == null ? relativePath : baseExecPath.getRelative(relativePath);
-    execPath = execPath.normalize();
     if (execPath.containsUplevelReferences()) {
       // Source exec paths cannot escape the source root.
       return null;
@@ -371,17 +354,16 @@ public synchronized Map<PathFragment, Artifact> resolveSourceArtifacts(
     ArrayList<PathFragment> unresolvedPaths = new ArrayList<>();
 
     for (PathFragment execPath : execPaths) {
-      PathFragment execPathNormalized = execPath.normalize();
-      if (execPathNormalized.containsUplevelReferences()) {
+      if (execPath.containsUplevelReferences()) {
         // Source exec paths cannot escape the source root.
         result.put(execPath, null);
         continue;
       }
       // First try a quick map lookup to see if the artifact already exists.
-      Artifact a = sourceArtifactCache.getArtifactIfValid(execPathNormalized);
+      Artifact a = sourceArtifactCache.getArtifactIfValid(execPath);
       if (a != null) {
         result.put(execPath, a);
-      } else if (isDerivedArtifact(execPathNormalized)) {
+      } else if (isDerivedArtifact(execPath)) {
         // Don't create an artifact if it's derived.
         result.put(execPath, null);
       } else {

src/main/java/com/google/devtools/build/lib/analysis/actions/PopulateTreeArtifactAction.java

@@ -273,7 +273,7 @@ private Iterable<PathFragment> readAndCheckManifestEntries()
       if (!line.isEmpty()) {
         PathFragment path = PathFragment.create(line);
 
-        if (!path.isNormalized() || path.isAbsolute()) {
+        if (!PathFragment.isNormalized(line) || path.isAbsolute()) {
           throw new IllegalManifestFileException(
               path + " is not a proper relative path");
         }