Have GlobFunction make use of the assumption that the glob's package exists by having it not declare a dep on the PackageLookupValue for the package. This optimization means that a BUILD file edit doesn't (necessarily) invalidate all the globs in the package; the PackageLookupValue node would get change-pruned but we still pay the very small cost of invalidating unnecessarily.

Also slightly improve variable naming in GlobFunctionTest.

--
MOS_MIGRATED_REVID=113799936

1 files changed, 8 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
index 5520c23621..50b7c294c7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
@@ -18,7 +18,6 @@ import com.google.common.cache.CacheBuilder;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.Dirent.Type;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -54,16 +53,8 @@ public final class GlobFunction implements SkyFunction {
   public SkyValue compute(SkyKey skyKey, Environment env) throws GlobFunctionException {
     GlobDescriptor glob = (GlobDescriptor) skyKey.argument();
 
-    PackageLookupValue globPkgLookupValue = (PackageLookupValue)
-        env.getValue(PackageLookupValue.key(glob.getPackageId()));
-    if (globPkgLookupValue == null) {
-      return null;
-    }
-    Preconditions.checkState(globPkgLookupValue.packageExists(), "%s isn't an existing package",
-        glob.getPackageId());
-    // Note that this implies that the package's BUILD file exists which implies that the
-    // package's directory exists.
-
+    // Note that the glob's package is assumed to exist which implies that the package's BUILD file
+    // exists which implies that the package's directory exists.
     PathFragment globSubdir = glob.getSubdir();
     if (!globSubdir.equals(PathFragment.EMPTY_FRAGMENT)) {
       PackageLookupValue globSubdirPkgLookupValue = (PackageLookupValue) env.getValue(
@@ -103,8 +94,8 @@ public final class GlobFunction implements SkyFunction {
           matches.add(globSubdir);
         }
       } else {
-        SkyKey globKey = GlobValue.internalKey(
-            glob.getPackageId(), globSubdir, patternTail, glob.excludeDirs());
+        SkyKey globKey = GlobValue.internalKey(glob.getPackageId(), glob.getPackageRoot(),
+            globSubdir, patternTail, glob.excludeDirs());
         GlobValue globValue = (GlobValue) env.getValue(globKey);
         if (globValue == null) {
           return null;
@@ -114,8 +105,7 @@ public final class GlobFunction implements SkyFunction {
     }
 
     PathFragment dirPathFragment = glob.getPackageId().getPackageFragment().getRelative(globSubdir);
-    RootedPath dirRootedPath = RootedPath.toRootedPath(globPkgLookupValue.getRoot(),
-        dirPathFragment);
+    RootedPath dirRootedPath = RootedPath.toRootedPath(glob.getPackageRoot(), dirPathFragment);
     if (alwaysUseDirListing || containsGlobs(patternHead)) {
       // Pattern contains globs, so a directory listing is required.
       //
@@ -145,7 +135,7 @@ public final class GlobFunction implements SkyFunction {
           // For symlinks, look up the corresponding FileValue. This ensures that if the symlink
           // changes and "switches types" (say, from a file to a directory), this value will be
           // invalidated.
-          RootedPath symlinkRootedPath = RootedPath.toRootedPath(globPkgLookupValue.getRoot(),
+          RootedPath symlinkRootedPath = RootedPath.toRootedPath(glob.getPackageRoot(),
               dirPathFragment.getRelative(fileName));
           FileValue symlinkFileValue = (FileValue) env.getValue(FileValue.key(symlinkRootedPath));
           if (symlinkFileValue == null) {
@@ -166,7 +156,7 @@ public final class GlobFunction implements SkyFunction {
     } else {
       // Pattern does not contain globs, so a direct stat is enough.
       String fileName = patternHead;
-      RootedPath fileRootedPath = RootedPath.toRootedPath(globPkgLookupValue.getRoot(),
+      RootedPath fileRootedPath = RootedPath.toRootedPath(glob.getPackageRoot(),
           dirPathFragment.getRelative(fileName));
       FileValue fileValue = (FileValue) env.getValue(FileValue.key(fileRootedPath));
       if (fileValue == null) {
@@ -212,7 +202,7 @@ public final class GlobFunction implements SkyFunction {
       Environment env) {
     if (isDirectory && subdirPattern != null) {
       // This is a directory, and the pattern covers files under that directory.
-      SkyKey subdirGlobKey = GlobValue.internalKey(glob.getPackageId(),
+      SkyKey subdirGlobKey = GlobValue.internalKey(glob.getPackageId(), glob.getPackageRoot(),
           glob.getSubdir().getRelative(fileName), subdirPattern, glob.excludeDirs());
       GlobValue subdirGlob = (GlobValue) env.getValue(subdirGlobKey);
       if (subdirGlob == null) {