Skip to content

Commit

Permalink
In PathFragment#getRelative, return the passed-in PathFragment wh…
Browse files Browse the repository at this point in the history
…en possible instead of creating a new instance.

This is possible when:

* The argument is an absolute path fragment.
* The base path is the empty path fragment.

PiperOrigin-RevId: 681455847
Change-Id: Ie7dd6687a4ebf6ad113f8cf4368ffd110316890b
  • Loading branch information
justinhorvitz authored and copybara-github committed Oct 2, 2024
1 parent cda2598 commit c5d5d5e
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 8 deletions.
20 changes: 12 additions & 8 deletions src/main/java/com/google/devtools/build/lib/vfs/PathFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
* <p>Mac and Windows path fragments are case insensitive.
*/
@Immutable
public abstract class PathFragment
public abstract sealed class PathFragment
implements Comparable<PathFragment>, FileType.HasFileType, PathStrippable {
private static final OsPathPolicy OS = OsPathPolicy.getFilePathOs();

Expand All @@ -62,6 +62,7 @@ public abstract class PathFragment
private static final char ADDITIONAL_SEPARATOR_CHAR = OS.additionalSeparator();

private final String normalizedPath;

// DON'T add more fields here unless you know what you are doing. Adding another field will
// increase the shallow heap of a PathFragment instance beyond the current value of 16 bytes.
// Blaze's heap typically has many instances.
Expand Down Expand Up @@ -116,7 +117,7 @@ public String getPathString() {
return normalizedPath;
}

public boolean isEmpty() {
public final boolean isEmpty() {
return normalizedPath.isEmpty();
}

Expand Down Expand Up @@ -193,7 +194,10 @@ public String getBaseName() {
*/
public PathFragment getRelative(PathFragment other) {
Preconditions.checkNotNull(other);
// Fast-path: The path fragment is already normal, use cheaper normalization check
if (isEmpty() || other.isAbsolute()) {
return other;
}
// The path fragment is already normal, use cheaper normalization check.
String otherStr = other.normalizedPath;
return getRelative(otherStr, other.getDriveStrLength(), OS.needsToNormalizeSuffix(otherStr));
}
Expand All @@ -210,7 +214,7 @@ public PathFragment getRelative(String other) {
}

private PathFragment getRelative(String other, int otherDriveStrLength, int normalizationLevel) {
if (normalizedPath.isEmpty()) {
if (isEmpty()) {
return create(other);
}
if (other.isEmpty()) {
Expand Down Expand Up @@ -281,7 +285,7 @@ public PathFragment getParentDirectory() {
}
} else {
if (lastSeparator == -1) {
if (!normalizedPath.isEmpty()) {
if (!isEmpty()) {
return EMPTY_FRAGMENT;
} else {
return null;
Expand Down Expand Up @@ -380,7 +384,7 @@ public boolean endsWith(PathFragment other) {
return false;
}
return normalizedPath.length() == other.normalizedPath.length()
|| other.normalizedPath.isEmpty()
|| other.isEmpty()
|| normalizedPath.charAt(normalizedPath.length() - other.normalizedPath.length() - 1)
== SEPARATOR_CHAR;
}
Expand Down Expand Up @@ -608,7 +612,7 @@ public ImmutableList<String> splitToListOfSegments() {

/** Returns the path string, or '.' if the path is empty. */
public String getSafePathString() {
return !normalizedPath.isEmpty() ? normalizedPath : ".";
return !isEmpty() ? normalizedPath : ".";
}

/**
Expand All @@ -622,7 +626,7 @@ public String getSafePathString() {
public String getCallablePathString() {
if (isAbsolute()) {
return normalizedPath;
} else if (normalizedPath.isEmpty()) {
} else if (isEmpty()) {
return ".";
} else if (normalizedPath.indexOf(SEPARATOR_CHAR) == -1) {
return "." + SEPARATOR_CHAR + normalizedPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,20 @@ public void testGetRelative() {
assertThat(create("a").getRelative(".").getPathString()).isEqualTo("a");
}

@Test
public void getRelative_absolutePathArgument_returnsSameInstance(
@TestParameter({"/c/d", "c/d"}) String basePath) {
PathFragment absolute = PathFragment.create("/a/b");
assertThat(PathFragment.create(basePath).getRelative(absolute)).isSameInstanceAs(absolute);
}

@Test
public void getRelative_emptyBasePath_returnsSameInstance(
@TestParameter({"/a/b", "a/b"}) String argument) {
PathFragment instance = PathFragment.create(argument);
assertThat(EMPTY_FRAGMENT.getRelative(instance)).isSameInstanceAs(instance);
}

@Test
public void testIsNormalizedRelativePath() {
assertThat(PathFragment.isNormalizedRelativePath("/a")).isFalse();
Expand Down

0 comments on commit c5d5d5e

Please sign in to comment.