From 90750e66344819a9fef4f0f0a633bc85e4d5d72d Mon Sep 17 00:00:00 2001
From: nharmata <nharmata@google.com>
Date: Thu, 15 Mar 2018 09:35:48 -0700
Subject: Optimize GC churn due to PackageFunction#getContainingDirectory.
 While I'm here, also slightly restructure the code in
 #handleLabelsCrossingSubpackagesAndPropagateInconsistentFilesystemExceptions
 to make it more readable (and defer PackageIdentifier allocations).

Alternatives considered: Add a PathFragment#getParentDirectoryOfRelative(String other) instance method, and use it in PackageFunction#getContainingDirectory. I thought the approach in this CL would be preferable to adding a specialized method like that to PathFragment.

RELNOTES: None
PiperOrigin-RevId: 189197855
---
 .../devtools/build/lib/skyframe/PackageFunction.java   | 18 +++++++++++++-----
 .../google/devtools/build/lib/vfs/PathFragment.java    | 10 ++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

(limited to 'src/main')

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index b7bd7de706..8a6f603ca7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -819,6 +819,7 @@ public class PackageFunction implements SkyFunction {
   private static void handleLabelsCrossingSubpackagesAndPropagateInconsistentFilesystemExceptions(
       Root pkgRoot, PackageIdentifier pkgId, Package.Builder pkgBuilder, Environment env)
       throws InternalInconsistentFilesystemException, InterruptedException {
+    PathFragment pkgDir = pkgId.getPackageFragment();
     Set<SkyKey> containingPkgLookupKeys = Sets.newHashSet();
     Map<Target, SkyKey> targetToKey = new HashMap<>();
     for (Target target : pkgBuilder.getTargets()) {
@@ -829,10 +830,10 @@ public class PackageFunction implements SkyFunction {
                 "Null pkg for label %s as path fragment %s in pkg %s",
                 target.getLabel(), target.getLabel().getPackageFragment(), pkgId));
       }
-      PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
-      if (dir.equals(pkgId.getPackageFragment())) {
+      if (dir.equals(pkgDir)) {
         continue;
       }
+      PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
       SkyKey key = ContainingPackageLookupValue.key(dirId);
       targetToKey.put(target, key);
       containingPkgLookupKeys.add(key);
@@ -840,10 +841,10 @@ public class PackageFunction implements SkyFunction {
     Map<Label, SkyKey> subincludeToKey = new HashMap<>();
     for (Label subincludeLabel : pkgBuilder.getSubincludeLabels()) {
       PathFragment dir = getContainingDirectory(subincludeLabel);
-      PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
-      if (dir.equals(pkgId.getPackageFragment())) {
+      if (dir.equals(pkgDir)) {
         continue;
       }
+      PackageIdentifier dirId = PackageIdentifier.create(pkgId.getRepository(), dir);
       SkyKey key = ContainingPackageLookupValue.key(dirId);
       subincludeToKey.put(subincludeLabel, key);
       containingPkgLookupKeys.add(ContainingPackageLookupValue.key(dirId));
@@ -887,7 +888,14 @@ public class PackageFunction implements SkyFunction {
   private static PathFragment getContainingDirectory(Label label) {
     PathFragment pkg = label.getPackageFragment();
     String name = label.getName();
-    return name.equals(".") ? pkg : pkg.getRelative(name).getParentDirectory();
+    if (name.equals(".")) {
+      return pkg;
+    }
+    if (PathFragment.isNormalizedRelativePath(name) && !PathFragment.containsSeparator(name)) {
+      // Optimize for the common case of a label like '//pkg:target'.
+      return pkg;
+    }
+    return pkg.getRelative(name).getParentDirectory();
   }
 
   @Nullable
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 614a44774b..cf90a32e85 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
@@ -137,6 +137,16 @@ public final class PathFragment
     return getRelative(otherStr, other.getDriveStrLength(), OS.needsToNormalizeSuffix(otherStr));
   }
 
+  public static boolean isNormalizedRelativePath(String path) {
+    int driveStrLength = OS.getDriveStrLength(path);
+    int normalizationLevel = OS.needsToNormalize(path);
+    return driveStrLength == 0 && normalizationLevel == OsPathPolicy.NORMALIZED;
+  }
+
+  public static boolean containsSeparator(String path) {
+    return path.lastIndexOf(SEPARATOR_CHAR) != -1;
+  }
+
   /**
    * Returns a {@link PathFragment} instance representing the relative path between this {@link
    * PathFragment} and the given path.
-- 
cgit v1.2.3