Adjust visibility and type declarations for ContainingPackageLookupValue$Key and GlobValue#key. PiperOrigin-RevId: 217709920
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java index 60be6d6..ab79b94 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java
@@ -38,6 +38,7 @@ import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.skyframe.ContainingPackageLookupValue; +import com.google.devtools.build.lib.skyframe.GlobDescriptor; import com.google.devtools.build.lib.skyframe.GlobValue; import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException; import com.google.devtools.build.lib.skyframe.PerBuildSyscallCache; @@ -50,12 +51,14 @@ import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.ValueOrException; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; @@ -247,34 +250,49 @@ } /** - * Returns the "path" type hinted inclusions for a given path. Callers are responsible for + * Returns the "path" type hinted inclusions for the given paths. Callers are responsible for * caching. */ - Collection<Artifact> getPathLevelHintedInclusions(PathFragment path, Environment env) - throws InterruptedException { - return getHintedInclusionsWithSkyframe(Rule.Type.PATH, path, env); + Collection<Artifact> getPathLevelHintedInclusions( + ImmutableList<PathFragment> paths, Environment env) throws InterruptedException { + return getHintedInclusionsWithSkyframe(Rule.Type.PATH, paths, env); } /** - * Performs the work of matching a given path against the hints and returns the matching files. - * This is semantically different from {@link #getHintedInclusionsLegacy} in that it will not - * cross package boundaries. + * Performs the work of matching the given paths against the hints and returns the matching + * files. This is semantically different from {@link #getHintedInclusionsLegacy} in that it will + * not cross package boundaries. */ private Collection<Artifact> getHintedInclusionsWithSkyframe( - Rule.Type type, PathFragment path, Environment env) throws InterruptedException { - String pathString = path.getPathString(); - if (!pathString.startsWith(ALLOWED_PREFIX)) { + Rule.Type type, ImmutableList<PathFragment> paths, Environment env) + throws InterruptedException { + ImmutableList<String> pathStrings = + paths.stream() + .map(PathFragment::getPathString) + .filter((p) -> p.startsWith(ALLOWED_PREFIX)) + .collect(ImmutableList.toImmutableList()); + if (pathStrings.isEmpty()) { return ImmutableList.of(); } // Delay creation until we know we need one. Use a TreeSet to make sure that the results are // sorted with a stable order and unique. Set<Artifact> hints = null; + List<ContainingPackageLookupValue.Key> rulePaths = new ArrayList<>(rules.size()); + List<String> findFilters = new ArrayList<>(rules.size()); for (Rule rule : rules) { if (type != rule.type) { continue; } - Matcher m = rule.pattern.matcher(pathString); - if (!m.matches()) { + String firstMatchPathString = null; + Matcher m = null; + for (String pathString : pathStrings) { + m = rule.pattern.matcher(pathString); + if (m.matches()) { + firstMatchPathString = pathString; + break; + } + } + if (firstMatchPathString == null) { continue; } if (hints == null) { @@ -282,7 +300,8 @@ } PathFragment relativePath = PathFragment.create(m.replaceFirst(rule.findRoot)); if (LOG_FINE) { - logger.fine("hint for " + rule.type + " " + pathString + " root: " + relativePath); + logger.fine( + "hint for " + rule.type + " " + firstMatchPathString + " root: " + relativePath); } if (!relativePath.getPathString().startsWith(ALLOWED_PREFIX)) { logger.warning( @@ -292,12 +311,24 @@ + ALLOWED_PREFIX); continue; } + rulePaths.add( + ContainingPackageLookupValue.key(PackageIdentifier.createInMainRepo(relativePath))); + findFilters.add(rule.findFilter); + } + Map<SkyKey, ValueOrException<NoSuchPackageException>> containingPackageLookupValues = + env.getValuesOrThrow(rulePaths, NoSuchPackageException.class); + if (env.valuesMissing()) { + return null; + } + List<GlobDescriptor> globKeys = new ArrayList<>(rulePaths.size()); + for (int i = 0; i < rulePaths.size(); i++) { ContainingPackageLookupValue containingPackageLookupValue; + ContainingPackageLookupValue.Key relativePathKey = rulePaths.get(i); + PathFragment relativePath = relativePathKey.argument().getPackageFragment(); try { containingPackageLookupValue = - (ContainingPackageLookupValue) env.getValueOrThrow(ContainingPackageLookupValue.key( - PackageIdentifier.createInMainRepo(relativePath)), - NoSuchPackageException.class); + (ContainingPackageLookupValue) + containingPackageLookupValues.get(relativePathKey).get(); } catch (NoSuchPackageException e) { logger.warning( "Unexpected exception when looking up containing package for " @@ -306,45 +337,46 @@ + e.getMessage()); continue; } - if (env.valuesMissing()) { - return null; - } if (!containingPackageLookupValue.hasContainingPackage()) { logger.warning(relativePath + " not contained in any package: skipping"); continue; } PathFragment packageFragment = containingPackageLookupValue.getContainingPackageName().getPackageFragment(); - SkyKey globKey; + String pattern = findFilters.get(i); try { - globKey = + globKeys.add( GlobValue.key( containingPackageLookupValue.getContainingPackageName(), containingPackageLookupValue.getContainingPackageRoot(), - rule.findFilter, + pattern, /* excludeDirs= */ true, - relativePath.relativeTo(packageFragment)); + relativePath.relativeTo(packageFragment))); } catch (InvalidGlobPatternException e) { - env.getListener().handle( - Event.warn("Error parsing pattern " + rule.findFilter + " for " + relativePath)); + env.getListener() + .handle(Event.warn("Error parsing pattern " + pattern + " for " + relativePath)); continue; } - GlobValue globValue = null; + } + Map<SkyKey, ValueOrException<IOException>> globResults = + env.getValuesOrThrow(globKeys, IOException.class); + if (env.valuesMissing()) { + return null; + } + for (Map.Entry<SkyKey, ValueOrException<IOException>> globEntry : globResults.entrySet()) { + GlobValue globValue; + GlobDescriptor globKey = (GlobDescriptor) globEntry.getKey(); + PathFragment packageFragment = globKey.getPackageId().getPackageFragment(); try { - globValue = - (GlobValue) env.getValueOrThrow(globKey, IOException.class); + globValue = (GlobValue) globEntry.getValue().get(); } catch (IOException e) { - logger.warning("Error getting hints for " + relativePath + ": " + e); + logger.warning("Error getting hints for " + packageFragment + ": " + e); continue; } - if (env.valuesMissing()) { - return null; - } for (PathFragment file : globValue.getMatches()) { hints.add( artifactFactory.getSourceArtifact( - packageFragment.getRelative(file), - containingPackageLookupValue.getContainingPackageRoot())); + packageFragment.getRelative(file), globKey.getPackageRoot())); } } return hints == null || hints.isEmpty() ? ImmutableList.<Artifact>of() : hints;
diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java index 931eadb..8e97737 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/LegacyIncludeScanner.java
@@ -484,18 +484,16 @@ Preconditions.checkArgument(mainSource == null || sources.contains(mainSource), "The main source '%s' is not part of '%s'", mainSource, sources); ImmutableSet.Builder<Artifact> pathHints = null; + SkyFunction.Environment env = actionExecutionContext.getEnvironmentForDiscoveringInputs(); if (parser.getHints() != null) { pathHints = ImmutableSet.builderWithExpectedSize(quoteIncludePaths.size()); - SkyFunction.Environment env = actionExecutionContext.getEnvironmentForDiscoveringInputs(); - for (PathFragment path : quoteIncludePaths) { - Collection<Artifact> artifacts = parser.getHints().getPathLevelHintedInclusions(path, env); - if (env.valuesMissing()) { - break; - } - pathHints.addAll(Preconditions.checkNotNull(artifacts, path)); + Collection<Artifact> artifacts = + parser.getHints().getPathLevelHintedInclusions(quoteIncludePaths, env); + if (!env.valuesMissing()) { + pathHints.addAll(Preconditions.checkNotNull(artifacts, quoteIncludePaths)); } } - if (actionExecutionContext.getEnvironmentForDiscoveringInputs().valuesMissing()) { + if (env.valuesMissing()) { throw new MissingDepException(); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java index c9a341e..f9cfb82 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.AbstractSkyKey; import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; /** @@ -83,9 +84,9 @@ return message; } - @AutoCodec.VisibleForSerialization + /** {@link SkyKey} for {@code ContainingPackageLookupValue}. */ @AutoCodec - static class Key extends AbstractSkyKey<PackageIdentifier> { + public static class Key extends AbstractSkyKey<PackageIdentifier> { private static final Interner<Key> interner = BlazeInterners.newWeakInterner(); private Key(PackageIdentifier arg) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java index e268135..bcc89fc 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java
@@ -23,7 +23,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.UnixGlob; -import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; /** A value corresponding to a glob. */ @@ -75,13 +74,13 @@ } /** - * Constructs a {@link SkyKey} for a glob lookup. {@code packageName} is assumed to be an existing - * package. Trying to glob into a non-package is undefined behavior. + * Constructs a {@link GlobDescriptor} for a glob lookup. {@code packageName} is assumed to be an + * existing package. Trying to glob into a non-package is undefined behavior. * * @throws InvalidGlobPatternException if the pattern is not valid. */ @ThreadSafe - public static SkyKey key( + public static GlobDescriptor key( PackageIdentifier packageId, Root packageRoot, String pattern, @@ -101,12 +100,12 @@ } /** - * Constructs a {@link SkyKey} for a glob lookup. + * Constructs a {@link GlobDescriptor} for a glob lookup. * * <p>Do not use outside {@code GlobFunction}. */ @ThreadSafe - static SkyKey internalKey( + static GlobDescriptor internalKey( PackageIdentifier packageId, Root packageRoot, PathFragment subdir, @@ -116,21 +115,6 @@ } /** - * Constructs a {@link SkyKey} for a glob lookup. - * - * <p>Do not use outside {@code GlobFunction}. - */ - @ThreadSafe - static SkyKey internalKey(GlobDescriptor glob, String subdirName) { - return internalKey( - glob.getPackageId(), - glob.getPackageRoot(), - glob.getSubdir().getRelative(subdirName), - glob.getPattern(), - glob.excludeDirs()); - } - - /** * An exception that indicates that a glob pattern is syntactically invalid. */ @ThreadSafe