From a729b9b4c3d7844a7d44934bf3365f92633c0a60 Mon Sep 17 00:00:00 2001 From: tomlu Date: Thu, 8 Feb 2018 15:32:00 -0800 Subject: 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 --- .../build/lib/skyframe/ActionTemplateExpansionFunctionTest.java | 3 --- .../google/devtools/build/lib/skyframe/ArtifactFunctionTest.java | 8 +------- .../devtools/build/lib/skyframe/FilesystemValueCheckerTest.java | 1 - .../lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java | 2 -- .../devtools/build/lib/skyframe/TimestampBuilderTestCase.java | 2 -- .../google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java | 2 -- .../devtools/build/lib/skyframe/TreeArtifactMetadataTest.java | 1 - 7 files changed, 1 insertion(+), 18 deletions(-) (limited to 'src/test/java/com/google/devtools/build/lib/skyframe') diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java index 7c864789ec..f8b6a9db3d 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunctionTest.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.actions.SpawnActionTemplate.Output import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.testutil.FoundationTestCase; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.EvaluationResult; @@ -205,9 +204,7 @@ public final class ActionTemplateExpansionFunctionTest extends FoundationTestCas private SpecialArtifact createTreeArtifact(String path) { PathFragment execPath = PathFragment.create("out").getRelative(path); - Path fullPath = rootDirectory.getRelative(execPath); return new SpecialArtifact( - fullPath, ArtifactRoot.asDerivedRoot(rootDirectory, rootDirectory.getRelative("out")), execPath, ArtifactOwner.NullArtifactOwner.INSTANCE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java index 1699fc25fc..21b0a10274 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java @@ -250,10 +250,8 @@ public class ArtifactFunctionTest extends ArtifactFunctionTestCase { private Artifact createDerivedArtifact(String path) { PathFragment execPath = PathFragment.create("out").getRelative(path); - Path fullPath = root.getRelative(execPath); Artifact output = new Artifact( - fullPath, ArtifactRoot.asDerivedRoot(root, root.getRelative("out")), execPath, ALL_OWNER); @@ -264,9 +262,7 @@ public class ArtifactFunctionTest extends ArtifactFunctionTestCase { private Artifact createMiddlemanArtifact(String path) { ArtifactRoot middlemanRoot = ArtifactRoot.middlemanRoot(middlemanPath, middlemanPath.getRelative("out")); - Path fullPath = middlemanRoot.getRoot().getRelative(path); - return new Artifact( - fullPath, middlemanRoot, middlemanRoot.getExecPath().getRelative(path), ALL_OWNER); + return new Artifact(middlemanRoot, middlemanRoot.getExecPath().getRelative(path), ALL_OWNER); } private SpecialArtifact createDerivedTreeArtifactWithAction(String path) { @@ -277,9 +273,7 @@ public class ArtifactFunctionTest extends ArtifactFunctionTestCase { private SpecialArtifact createDerivedTreeArtifactOnly(String path) { PathFragment execPath = PathFragment.create("out").getRelative(path); - Path fullPath = root.getRelative(execPath); return new SpecialArtifact( - fullPath, ArtifactRoot.asDerivedRoot(root, root.getRelative("out")), execPath, ALL_OWNER, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index f7a989a4ae..48ccd7e341 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -620,7 +620,6 @@ public class FilesystemValueCheckerTest { outputDir.createDirectory(); ArtifactRoot derivedRoot = ArtifactRoot.asDerivedRoot(fs.getPath("/"), outputDir); return new SpecialArtifact( - outputPath, derivedRoot, derivedRoot.getExecPath().getRelative(derivedRoot.getRoot().relativize(outputPath)), ArtifactOwner.NullArtifactOwner.INSTANCE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index 872de47ad0..25b510e18e 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java @@ -171,10 +171,8 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe private Artifact derivedArtifact(String path) { PathFragment execPath = PathFragment.create("out").getRelative(path); - Path fullPath = rootDirectory.getRelative(execPath); Artifact output = new Artifact( - fullPath, ArtifactRoot.asDerivedRoot(rootDirectory, rootDirectory.getRelative("out")), execPath); return output; diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 3b39d03441..e63d1ea06e 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java @@ -336,9 +336,7 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { Artifact createDerivedArtifact(FileSystem fs, String name) { Path execRoot = fs.getPath(TestUtils.tmpDir()); PathFragment execPath = PathFragment.create("out").getRelative(name); - Path path = execRoot.getRelative(execPath); return new Artifact( - path, ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")), execPath, ACTION_LOOKUP_KEY); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java index 30b6c4caf8..5d24b9ef28 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactBuildTest.java @@ -1192,9 +1192,7 @@ public class TreeArtifactBuildTest extends TimestampBuilderTestCase { FileSystem fs = scratch.getFileSystem(); Path execRoot = fs.getPath(TestUtils.tmpDir()); PathFragment execPath = PathFragment.create("out").getRelative(name); - Path path = execRoot.getRelative(execPath); return new SpecialArtifact( - path, ArtifactRoot.asDerivedRoot(execRoot, execRoot.getRelative("out")), execPath, ACTION_LOOKUP_KEY, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java index 96bc4bed65..9e3fd39ddd 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java @@ -204,7 +204,6 @@ public class TreeArtifactMetadataTest extends ArtifactFunctionTestCase { Path fullPath = root.getRelative(execPath); SpecialArtifact output = new SpecialArtifact( - fullPath, ArtifactRoot.asDerivedRoot(root, root.getRelative("out")), execPath, ALL_OWNER, -- cgit v1.2.3