Skip to content

Commit

Permalink
Don't intern RootedPath on Windows.
Browse files Browse the repository at this point in the history
It remains to be investigated why instances that compare equal can't be used interchangeably and what test coverage we're lacking. See issue bazelbuild#17904.

PiperOrigin-RevId: 520099860
Change-Id: Ifa64b662658add884e64bc4b8ae42a1261fec4c1
  • Loading branch information
justinhorvitz authored and copybara-github committed Mar 28, 2023
1 parent 98bd4ae commit aaf1d06
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 6 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/vfs/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ java_library(
exclude = PATH_FRAGMENT_SOURCES + OUTPUT_SERVICE_SOURCES + OS_PATH_POLICY_SOURCES,
),
deps = [
":ospathpolicy",
":pathfragment",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/concurrent",
Expand Down
20 changes: 14 additions & 6 deletions src/main/java/com/google/devtools/build/lib/vfs/RootedPath.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,21 @@
* <p>Two {@link RootedPath}s are considered equal iff they have equal roots and equal relative
* paths.
*
* <p>Instances are interned, which results in a large memory benefit (see cl/516855266). In
* addition to being a {@link SkyKey} itself, {@link RootedPath} is used as a field in several other
* common {@link SkyKey} types. Interning on the level of those keys does not deduplicate referenced
* {@link RootedPath} instances which are also used as a {@link SkyKey} directly.
* <p>Instances are interned (except on Windows), which results in a large memory benefit (see
* cl/516855266). In addition to being a {@link SkyKey} itself, {@link RootedPath} is used as a
* field in several other common {@link SkyKey} types. Interning on the level of those keys does not
* deduplicate referenced {@link RootedPath} instances which are also used as a {@link SkyKey}
* directly.
*/
@AutoCodec
public final class RootedPath implements Comparable<RootedPath>, FileStateKey {

private static final SkyKeyInterner<RootedPath> interner = SkyKey.newInterner();
// Interning on Windows (case-insensitive) surfaces a bug where paths that only differ in casing
// use the same RootedPath instance.
// TODO(#17904): Investigate this bug and add test coverage.
@Nullable
private static final SkyKeyInterner<RootedPath> interner =
OsPathPolicy.getFilePathOs().isCaseSensitive() ? SkyKey.newInterner() : null;

private final Root root;
private final PathFragment rootRelativePath;
Expand All @@ -52,7 +58,8 @@ static RootedPath createInternal(Root root, PathFragment rootRelativePath) {
"rootRelativePath: %s root: %s",
rootRelativePath,
root);
return interner.intern(new RootedPath(root, rootRelativePath));
var rootedPath = new RootedPath(root, rootRelativePath);
return interner != null ? interner.intern(rootedPath) : rootedPath;
}

private RootedPath(Root root, PathFragment rootRelativePath) {
Expand Down Expand Up @@ -153,6 +160,7 @@ public RootedPath argument() {
}

@Override
@Nullable
public SkyKeyInterner<?> getSkyKeyInterner() {
return interner;
}
Expand Down

0 comments on commit aaf1d06

Please sign in to comment.