diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index 267918296d6fc7..31a8ece4011673 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; import com.google.common.annotations.VisibleForTesting; @@ -24,8 +25,6 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.common.collect.Streams; import com.google.common.eventbus.EventBus; @@ -72,7 +71,6 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildMetrics.BuildGraphMetrics; import com.google.devtools.build.lib.causes.AnalysisFailedCause; import com.google.devtools.build.lib.cmdline.Label; -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.collect.nestedset.Order; @@ -90,12 +88,12 @@ import com.google.devtools.build.lib.skyframe.AspectKeyCreator.TopLevelAspectsKey; import com.google.devtools.build.lib.skyframe.BuildDriverKey.TestType; import com.google.devtools.build.lib.skyframe.SkyframeErrorProcessor.ErrorProcessingResult; +import com.google.devtools.build.lib.skyframe.SkyframeExecutor.ConfigureTargetsResult; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.FailureToRetrieveIntrospectedValueException; import com.google.devtools.build.lib.skyframe.SkyframeExecutor.TopLevelActionConflictReport; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.DetailedExitCode.DetailedExitCodeComparator; import com.google.devtools.build.lib.util.OrderedSetMultimap; -import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.ErrorInfo; import com.google.devtools.build.skyframe.EvaluationProgressReceiver; import com.google.devtools.build.skyframe.EvaluationResult; @@ -353,7 +351,7 @@ public SkyframeAnalysisResult configureTargets( int cpuHeavySkyKeysThreadPoolSize) throws InterruptedException, ViewCreationFailedException { enableAnalysis(true); - EvaluationResult result; + ConfigureTargetsResult result; try (SilentCloseable c = Profiler.instance().profile("skyframeExecutor.configureTargets")) { result = skyframeExecutor.configureTargets( @@ -367,49 +365,11 @@ public SkyframeAnalysisResult configureTargets( enableAnalysis(false); } - int numOfAspects = 0; - if (!topLevelAspectsKeys.isEmpty()) { - numOfAspects = - topLevelAspectsKeys.size() - * topLevelAspectsKeys.get(0).getTopLevelAspectsClasses().size(); - } - Map aspects = Maps.newHashMapWithExpectedSize(numOfAspects); - Root singleSourceRoot = skyframeExecutor.getForcedSingleSourceRootIfNoExecrootSymlinkCreation(); - NestedSetBuilder packages = - singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null; - ImmutableList.Builder aspectKeysBuilder = ImmutableList.builder(); - - for (TopLevelAspectsKey key : topLevelAspectsKeys) { - TopLevelAspectsValue value = (TopLevelAspectsValue) result.get(key); - if (value == null) { - // Skip aspects that couldn't be applied to targets. - continue; - } - for (AspectValue aspectValue : value.getTopLevelAspectsValues()) { - aspects.put(aspectValue.getKey(), aspectValue.getConfiguredAspect()); - if (packages != null) { - packages.addTransitive(Preconditions.checkNotNull(aspectValue.getTransitivePackages())); - } - aspectKeysBuilder.add(aspectValue.getKey()); - } - } - ImmutableList aspectKeys = aspectKeysBuilder.build(); - - Collection cts = Lists.newArrayListWithCapacity(ctKeys.size()); - for (ConfiguredTargetKey value : ctKeys) { - ConfiguredTargetValue ctValue = (ConfiguredTargetValue) result.get(value); - if (ctValue == null) { - continue; - } - cts.add(ctValue.getConfiguredTarget()); - if (packages != null) { - packages.addTransitive(Preconditions.checkNotNull(ctValue.getTransitivePackages())); - } - } - PackageRoots packageRoots = - singleSourceRoot == null - ? new MapAsPackageRoots(collectPackageRoots(packages.build().toList())) - : new PackageRootsNoSymlinkCreation(singleSourceRoot); + ImmutableSet cts = result.configuredTargets(); + ImmutableMap aspects = result.aspects(); + ImmutableSet aspectKeys = aspects.keySet(); + PackageRoots packageRoots = result.packageRoots(); + EvaluationResult evaluationResult = result.evaluationResult(); ImmutableMap actionConflicts = ImmutableMap.of(); try (SilentCloseable c = @@ -444,20 +404,20 @@ public SkyframeAnalysisResult configureTargets( } foundActionConflictInLatestCheck = !actionConflicts.isEmpty(); - if (!result.hasError() && !foundActionConflictInLatestCheck) { + if (!evaluationResult.hasError() && !foundActionConflictInLatestCheck) { return new SkyframeAnalysisResult( - /*hasLoadingError=*/ false, - /*hasAnalysisError=*/ false, + /* hasLoadingError= */ false, + /* hasAnalysisError= */ false, foundActionConflictInLatestCheck, - ImmutableSet.copyOf(cts), - result.getWalkableGraph(), - ImmutableMap.copyOf(aspects), + cts, + evaluationResult.getWalkableGraph(), + aspects, packageRoots); } ErrorProcessingResult errorProcessingResult = SkyframeErrorProcessor.processAnalysisErrors( - result, + evaluationResult, configurationLookupSupplier, skyframeExecutor.getCyclesReporter(), eventHandler, @@ -553,23 +513,23 @@ public SkyframeAnalysisResult configureTargets( .filter(topLevelActionConflictReport::isErrorFree) .map( k -> - Preconditions.checkNotNull((ConfiguredTargetValue) result.get(k), k) + Preconditions.checkNotNull((ConfiguredTargetValue) evaluationResult.get(k), k) .getConfiguredTarget()) - .collect(toImmutableList()); + .collect(toImmutableSet()); aspects = aspects.entrySet().stream() .filter(e -> topLevelActionConflictReport.isErrorFree(e.getKey())) - .collect(ImmutableMap.toImmutableMap(Entry::getKey, Entry::getValue)); + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); } return new SkyframeAnalysisResult( errorProcessingResult.hasLoadingError(), - result.hasError() || foundActionConflictInLatestCheck, + evaluationResult.hasError() || foundActionConflictInLatestCheck, foundActionConflictInLatestCheck, - ImmutableSet.copyOf(cts), - result.getWalkableGraph(), - ImmutableMap.copyOf(aspects), + cts, + evaluationResult.getWalkableGraph(), + aspects, packageRoots); } @@ -1165,19 +1125,6 @@ private boolean shouldCheckForConflicts( return false; } - /** Returns a map of collected package names to root paths. */ - private static ImmutableMap collectPackageRoots( - Collection packages) { - // Make a map of the package names to their root paths. - ImmutableMap.Builder packageRoots = ImmutableMap.builder(); - for (Package pkg : packages) { - if (pkg.getSourceRoot().isPresent()) { - packageRoots.put(pkg.getPackageIdentifier(), pkg.getSourceRoot().get()); - } - } - return packageRoots.buildOrThrow(); - } - public ArtifactFactory getArtifactFactory() { return artifactFactory; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index c5eb337632fdc5..3d65ace3577807 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -22,6 +22,7 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; +import com.google.auto.value.AutoValue; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Functions; import com.google.common.base.Preconditions; @@ -68,6 +69,7 @@ import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.MapBasedActionGraph; import com.google.devtools.build.lib.actions.MetadataProvider; +import com.google.devtools.build.lib.actions.PackageRoots; import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.ThreadStateReceiver; import com.google.devtools.build.lib.actions.UserExecException; @@ -117,6 +119,8 @@ import com.google.devtools.build.lib.cmdline.RepositoryMapping; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.cmdline.TargetParsingException; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.NestedSetVisitor; import com.google.devtools.build.lib.concurrent.ExecutorUtil; import com.google.devtools.build.lib.concurrent.NamedForkJoinPool; @@ -2195,7 +2199,7 @@ protected abstract void invalidateFilesUnderPathForTestingImpl( /** Configures a given set of configured targets. */ @CanIgnoreReturnValue - EvaluationResult configureTargets( + ConfigureTargetsResult configureTargets( ExtendedEventHandler eventHandler, ImmutableList configuredTargetKeys, ImmutableList topLevelAspectKeys, @@ -2219,7 +2223,68 @@ EvaluationResult configureTargets( memoizingEvaluator.evaluate( analysisPhaseKeys(configuredTargetKeys, topLevelAspectKeys), evaluationContext); perCommandSyscallCache.noteAnalysisPhaseEnded(); - return result; + + ImmutableSet.Builder configuredTargets = ImmutableSet.builder(); + ImmutableMap.Builder aspects = ImmutableMap.builder(); + Root singleSourceRoot = getForcedSingleSourceRootIfNoExecrootSymlinkCreation(); + NestedSetBuilder packages = + singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null; + + for (ConfiguredTargetKey key : configuredTargetKeys) { + ConfiguredTargetValue value = (ConfiguredTargetValue) result.get(key); + if (value == null) { + continue; + } + configuredTargets.add(value.getConfiguredTarget()); + if (packages != null) { + packages.addTransitive(value.getTransitivePackages()); + } + } + + for (TopLevelAspectsKey key : topLevelAspectKeys) { + TopLevelAspectsValue value = (TopLevelAspectsValue) result.get(key); + if (value == null) { + continue; // Skip aspects that couldn't be applied to targets. + } + for (AspectValue aspectValue : value.getTopLevelAspectsValues()) { + aspects.put(aspectValue.getKey(), aspectValue.getConfiguredAspect()); + if (packages != null) { + packages.addTransitive(aspectValue.getTransitivePackages()); + } + } + } + + PackageRoots packageRoots = + singleSourceRoot == null + ? new MapAsPackageRoots(collectPackageRoots(packages.build())) + : new PackageRootsNoSymlinkCreation(singleSourceRoot); + + return new AutoValue_SkyframeExecutor_ConfigureTargetsResult( + result, configuredTargets.build(), aspects.buildOrThrow(), packageRoots); + } + + /** Result of a call to {@link #configureTargets}. */ + @AutoValue + abstract static class ConfigureTargetsResult { + abstract EvaluationResult evaluationResult(); + + abstract ImmutableSet configuredTargets(); + + abstract ImmutableMap aspects(); + + abstract PackageRoots packageRoots(); + } + + /** Returns a map of collected package names to root paths. */ + private static ImmutableMap collectPackageRoots( + NestedSet packages) { + ImmutableMap.Builder packageRoots = ImmutableMap.builder(); + for (Package pkg : packages.toList()) { + if (pkg.getSourceRoot().isPresent()) { + packageRoots.put(pkg.getPackageIdentifier(), pkg.getSourceRoot().get()); + } + } + return packageRoots.buildOrThrow(); } /** @@ -3079,7 +3144,7 @@ protected ExecutionFinishedEvent.Builder createExecutionFinishedEventInternal() } final AnalysisTraversalResult getActionLookupValuesInBuild( - List topLevelCtKeys, ImmutableList aspectKeys) + List topLevelCtKeys, ImmutableSet aspectKeys) throws InterruptedException { try (SilentCloseable c = Profiler.instance().profile("skyframeExecutor.getActionLookupValuesInBuild")) {