Fix usages of PathFragment segments that will become inefficient.
An upcoming replacement to PathFragment will not have efficient segment semantics, causing code to become unnecessarily inefficient.
RELNOTES: None
PiperOrigin-RevId: 182553098
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
index 72d2637..eb9165a 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Artifact.java
@@ -200,7 +200,7 @@
throw new IllegalArgumentException(execPath + ": illegal execPath for " + path
+ " (root: " + root + ")");
}
- if (execPath.segmentCount() == 0) {
+ if (execPath.isEmpty()) {
throw new IllegalArgumentException(
"it is illegal to create an artifact with an empty execPath");
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
index 1aba67c..7382944 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java
@@ -298,7 +298,7 @@
baseExecPath,
baseRoot);
Preconditions.checkState(
- relativePath.segmentCount() > 0, "%s %s %s", relativePath, baseExecPath, baseRoot);
+ !relativePath.isEmpty(), "%s %s %s", relativePath, baseExecPath, baseRoot);
PathFragment execPath =
baseExecPath == null ? relativePath : baseExecPath.getRelative(relativePath);
execPath = execPath.normalize();
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
index 71449aa..3363dc3 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactRoot.java
@@ -61,7 +61,8 @@
return asSourceRoot(packageRoot);
} else {
Path actualRootPath = packageRoot.asPath();
- for (int i = 0; i < repository.getSourceRoot().segmentCount(); i++) {
+ int segmentCount = repository.getSourceRoot().segmentCount();
+ for (int i = 0; i < segmentCount; i++) {
actualRootPath = actualRootPath.getParentDirectory();
}
return asSourceRoot(Root.fromPath(actualRootPath));
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
index 544f09f..89fc76b 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
@@ -123,9 +123,7 @@
ImmutableList<FilesetTraversalParams> nested,
PathFragment destDir,
@Nullable Set<String> excludes) {
- if (nested.size() == 1
- && destDir.segmentCount() == 0
- && (excludes == null || excludes.isEmpty())) {
+ if (nested.size() == 1 && destDir.isEmpty() && (excludes == null || excludes.isEmpty())) {
// Wrapping the traversal here would not lead to a different result: the output location is
// the same and there are no additional excludes.
return Iterables.getOnlyElement(nested);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index cc28554..b50259e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -794,9 +794,10 @@
@Override
public PathFragment getDefaultJavaResourcePath(PathFragment path) {
// Look for src/.../resources to match Maven repository structure.
- for (int i = 0; i < path.segmentCount() - 2; ++i) {
- if (path.getSegment(i).equals("src") && path.getSegment(i + 2).equals("resources")) {
- return path.subFragment(i + 3, path.segmentCount());
+ List<String> segments = path.getSegments();
+ for (int i = 0; i < segments.size() - 2; ++i) {
+ if (segments.get(i).equals("src") && segments.get(i + 2).equals("resources")) {
+ return path.subFragment(i + 3);
}
}
PathFragment javaPath = JavaUtil.getJavaPath(path);
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
index 13b57cf..4ed8fda 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SymlinkForest.java
@@ -123,7 +123,8 @@
continue;
}
Root pkgRoot = entry.getValue();
- for (int i = 1; i <= pkgId.getPackageFragment().segmentCount(); i++) {
+ int segmentCount = pkgId.getPackageFragment().segmentCount();
+ for (int i = 1; i <= segmentCount; i++) {
PackageIdentifier dir = createInRepo(pkgId, pkgId.getPackageFragment().subFragment(0, i));
Set<Root> roots = dirRootsMap.computeIfAbsent(dir, k -> Sets.newHashSet());
roots.add(pkgRoot);
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index 9469ddd..ac62a3a 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -86,10 +86,10 @@
if (tofind.startsWith(Label.EXTERNAL_PATH_PREFIX)) {
// TODO(ulfjack): Remove this when kchodorow@'s exec root rearrangement has been rolled out.
RepositoryName repository = RepositoryName.create("@" + tofind.getSegment(1));
- return PackageIdentifier.create(repository, tofind.subFragment(2, tofind.segmentCount()));
+ return PackageIdentifier.create(repository, tofind.subFragment(2));
} else if (!tofind.normalize().isNormalized()) {
RepositoryName repository = RepositoryName.create("@" + tofind.getSegment(1));
- return PackageIdentifier.create(repository, tofind.subFragment(2, tofind.segmentCount()));
+ return PackageIdentifier.create(repository, tofind.subFragment(2));
} else {
return PackageIdentifier.createInMainRepo(tofind);
}
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
index b008978..94705d7 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.util.StringCanonicalizer;
import com.google.devtools.build.lib.util.StringUtilities;
import com.google.devtools.build.lib.vfs.PathFragment;
-
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
@@ -144,7 +143,7 @@
}
try {
RepositoryName repoName = RepositoryName.create("@" + path.getSegment(1));
- PathFragment subPath = path.subFragment(2, path.segmentCount());
+ PathFragment subPath = path.subFragment(2);
return Pair.of(repoName, subPath);
} catch (LabelSyntaxException e) {
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
index 49865e7..319ea6b 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/FilesetManifest.java
@@ -128,7 +128,7 @@
"fileset manifest line must start with '%s': '%s'", workspaceName, location));
} else {
// Erase "<workspaceName>/" prefix.
- location = location.subFragment(1, location.segmentCount());
+ location = location.subFragment(1);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index ca12997..c37ffa3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1281,7 +1281,8 @@
for (OutputFile outputFile : rule.getOutputFiles()) {
targets.put(outputFile.getName(), outputFile);
PathFragment outputFileFragment = PathFragment.create(outputFile.getName());
- for (int i = 1; i < outputFileFragment.segmentCount(); i++) {
+ int segmentCount = outputFileFragment.segmentCount();
+ for (int i = 1; i < segmentCount; i++) {
String prefix = outputFileFragment.subFragment(0, i).toString();
outputFilePrefixes.putIfAbsent(prefix, outputFile);
}
@@ -1473,7 +1474,8 @@
// Check if a prefix of this output file matches an already existing one
PathFragment outputFileFragment = PathFragment.create(outputFileName);
- for (int i = 1; i < outputFileFragment.segmentCount(); i++) {
+ int segmentCount = outputFileFragment.segmentCount();
+ for (int i = 1; i < segmentCount; i++) {
String prefix = outputFileFragment.subFragment(0, i).toString();
if (outputFiles.containsKey(prefix)) {
throw conflictingOutputFile(outputFile, outputFiles.get(prefix));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
index be9bbf1..6c8e040 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java
@@ -324,30 +324,32 @@
if (needle.equals(PathFragment.EMPTY_FRAGMENT)) {
return haystack;
}
+ List<String> needleSegments = needle.getSegments();
// Compute the overlap offset for duplicated parts of the needle.
- int[] overlap = new int[needle.segmentCount() + 1];
+ int[] overlap = new int[needleSegments.size() + 1];
// Start overlap at -1, as it will cancel out the increment in the search.
// See http://en.wikipedia.org/wiki/Knuth%E2%80%93Morris%E2%80%93Pratt_algorithm for the
// details.
overlap[0] = -1;
- for (int i = 0, j = -1; i < needle.segmentCount(); j++, i++, overlap[i] = j) {
- while (j >= 0 && !needle.getSegment(i).equals(needle.getSegment(j))) {
+ for (int i = 0, j = -1; i < needleSegments.size(); j++, i++, overlap[i] = j) {
+ while (j >= 0 && !needleSegments.get(i).equals(needleSegments.get(j))) {
// Walk the overlap until the bound is found.
j = overlap[j];
}
}
// TODO(corysmith): reverse the search algorithm.
// Keep the index of the found so that the rightmost index is taken.
+ List<String> haystackSegments = haystack.getSegments();
int found = -1;
- for (int i = 0, j = 0; i < haystack.segmentCount(); i++) {
+ for (int i = 0, j = 0; i < haystackSegments.size(); i++) {
- while (j >= 0 && !haystack.getSegment(i).equals(needle.getSegment(j))) {
+ while (j >= 0 && !haystackSegments.get(i).equals(needleSegments.get(j))) {
// Not matching, walk the needle index to attempt another match.
j = overlap[j];
}
j++;
// Needle index is exhausted, so the needle must match.
- if (j == needle.segmentCount()) {
+ if (j == needleSegments.size()) {
// Record the found index + 1 to be inclusive of the end index.
found = i + 1;
// Subtract one from the needle index to restart the search process
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
index f11cd18..c754d9e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/LocalResourceContainer.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Arrays;
import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Optional;
import java.util.Set;
import javax.annotation.Nullable;
@@ -142,7 +143,8 @@
PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment);
if (packageRelativePath.startsWith(assetsDir)) {
PathFragment relativePath = packageRelativePath.relativeTo(assetsDir);
- assetRoots.add(trimTail(file.getExecPath(), relativePath));
+ PathFragment path = file.getExecPath();
+ assetRoots.add(path.subFragment(0, path.segmentCount() - relativePath.segmentCount()));
} else {
ruleContext.attributeError(
ResourceType.ASSETS.getAttribute(),
@@ -273,8 +275,11 @@
file.getArtifactOwner().getLabel().getPackageIdentifier().getSourceRoot();
PathFragment packageRelativePath = file.getRootRelativePath().relativeTo(packageFragment);
try {
+ PathFragment path = file.getExecPath();
resourceRoots.add(
- trimTail(file.getExecPath(), makeRelativeTo(resourceDir, packageRelativePath)));
+ path.subFragment(
+ 0,
+ path.segmentCount() - segmentCountAfterAncestor(resourceDir, packageRelativePath)));
} catch (IllegalArgumentException e) {
ruleErrorConsumer.attributeError(
resourcesAttr,
@@ -304,7 +309,7 @@
}
// TODO(bazel-team): Expand Fileset to verify, or remove Fileset as an option for resources.
if (artifact.isFileset() || artifact.isTreeArtifact()) {
- return fragment.subFragment(segmentCount - 1, segmentCount);
+ return fragment.subFragment(segmentCount - 1);
}
// Check the resource folder type layout.
@@ -320,23 +325,20 @@
return fragment.subFragment(segmentCount - 3, segmentCount - 2);
}
- /**
- * Returns the root-part of a given path by trimming off the end specified by a given tail.
- * Assumes that the tail is known to match, and simply relies on the segment lengths.
- */
- private static PathFragment trimTail(PathFragment path, PathFragment tail) {
- return path.subFragment(0, path.segmentCount() - tail.segmentCount());
- }
-
- private static PathFragment makeRelativeTo(PathFragment ancestor, PathFragment path) {
+ private static int segmentCountAfterAncestor(PathFragment ancestor, PathFragment path) {
String cutAtSegment = ancestor.getSegment(ancestor.segmentCount() - 1);
- int totalPathSegments = path.segmentCount() - 1;
- for (int i = totalPathSegments; i >= 0; i--) {
- if (path.getSegment(i).equals(cutAtSegment)) {
- return path.subFragment(i, totalPathSegments);
+ int index = -1;
+ List<String> segments = path.getSegments();
+ for (int i = segments.size() - 1; i >= 0; i--) {
+ if (segments.get(i).equals(cutAtSegment)) {
+ index = i;
+ break;
}
}
- throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor);
+ if (index == -1) {
+ throw new IllegalArgumentException("PathFragment " + path + " is not beneath " + ancestor);
+ }
+ return segments.size() - index - 1;
}
private final ImmutableList<Artifact> resources;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 6737c6c..7b54954 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -487,7 +487,7 @@
ruleContext.attributeError("includes",
"Path references a path above the execution root.");
}
- if (includesPath.segmentCount() == 0) {
+ if (includesPath.isEmpty()) {
ruleContext.attributeError(
"includes",
"'"
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 7bede6c..284be8c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -893,7 +893,7 @@
}
// Need to do dir/package matching: first try a quick exact lookup.
PathFragment includeDir = input.getRootRelativePath().getParentDirectory();
- if (includeDir.segmentCount() == 0 || declaredIncludeDirs.contains(includeDir)) {
+ if (includeDir.isEmpty() || declaredIncludeDirs.contains(includeDir)) {
return true; // OK: quick exact match.
}
// Not found in the quick lookup: try the wildcards.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java
index eaddad0..ee01342 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaUtil.java
@@ -157,7 +157,7 @@
public static PathFragment getJavaPath(PathFragment path) {
int index = javaSegmentIndex(path);
if (index >= 0) {
- return path.subFragment(index + 1, path.segmentCount());
+ return path.subFragment(index + 1);
}
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
index 024287d..2b15ced 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java
@@ -509,7 +509,7 @@
throws IOException, InterruptedException {
Path externalRepoDir = getExternalRepositoryDirectory(directories);
PathFragment repositoryPath = rootedPath.asPath().relativeTo(externalRepoDir);
- if (repositoryPath.segmentCount() == 0) {
+ if (repositoryPath.isEmpty()) {
// We are the top of the repository path (<outputBase>/external), not in an actual external
// repository path.
return;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
index 4bf54fc..bedd9ba 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
@@ -149,7 +149,7 @@
DirectoryTree root = new DirectoryTree();
for (ResolvedFile f : rftv.getTransitiveFiles().toCollection()) {
PathFragment path = f.getNameInSymlinkTree().relativeTo(prefixToRemove);
- if (path.segmentCount() > 0) {
+ if (!path.isEmpty()) {
path = t.getDestPath().getRelative(path);
DirectoryTree dir = root;
for (int i = 0; i < path.segmentCount() - 1; ++i) {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java b/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
index f385831..9db3aa3 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
@@ -612,6 +612,20 @@
}
/**
+ * Returns a new path fragment that is a sub fragment of this one. The sub fragment begins at the
+ * specified <code>beginIndex</code> segment and contains the rest of the original path fragment.
+ *
+ * @param beginIndex the beginning index, inclusive.
+ * @return the specified sub fragment, never null.
+ * @exception IndexOutOfBoundsException if the <code>beginIndex</code> is negative, or <code>
+ * endIndex</code> is larger than the length of this <code>String</code> object, or <code>
+ * beginIndex</code> is larger than <code>endIndex</code>.
+ */
+ public PathFragment subFragment(int beginIndex) {
+ return subFragment(beginIndex, segments.length);
+ }
+
+ /**
* Returns true iff the path represented by this object is absolute.
*
* <p>True both for UNIX-style absolute paths ("/foo") and Windows-style ("C:/foo"). False for a
@@ -642,6 +656,10 @@
// windowsVolume).
public abstract char getDriveLetter();
+ public boolean isEmpty() {
+ return segments.length == 0;
+ }
+
/**
* Returns the number of segments in this path.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/WindowsPathFragment.java b/src/main/java/com/google/devtools/build/lib/vfs/WindowsPathFragment.java
index 928c50d..ac3be0f 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/WindowsPathFragment.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/WindowsPathFragment.java
@@ -237,10 +237,6 @@
&& HELPER.segmentsEqual(this.segments, otherRelativeWindowsPathFragment.segments);
}
- private boolean isEmpty() {
- return segmentCount() == 0;
- }
-
// Java serialization looks for the presence of this method in the concrete class. It is not
// inherited from the parent class.
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
index e63d03f..8fd2118 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java
@@ -377,8 +377,9 @@
if (traversals > MAX_TRAVERSALS) {
throw Error.ELOOP.exception(path);
}
- for (int ii = linkTarget.segmentCount() - 1; ii >= 0; --ii) {
- stack.push(linkTarget.getSegment(ii)); // Note this may include ".." segments.
+ List<String> segments = linkTarget.getSegments();
+ for (int ii = segments.size() - 1; ii >= 0; --ii) {
+ stack.push(segments.get(ii)); // Note this may include ".." segments.
}
} else {
inode = child;
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java b/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java
index e6d1cc7..7fa3c73 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/PathFragmentTest.java
@@ -340,6 +340,8 @@
assertPath("/", PathFragment.create("/foo/bar/baz").subFragment(0, 0));
assertPath("", PathFragment.create("foo/bar/baz").subFragment(0, 0));
assertPath("", PathFragment.create("foo/bar/baz").subFragment(1, 1));
+ assertPath("/foo/bar/baz", PathFragment.create("/foo/bar/baz").subFragment(0));
+ assertPath("bar/baz", PathFragment.create("/foo/bar/baz").subFragment(1));
try {
fail("unexpectedly succeeded: " + PathFragment.create("foo/bar/baz").subFragment(3, 2));
} catch (IndexOutOfBoundsException e) { /* Expected. */ }
