Skip to content

Commit

Permalink
Move some processing of the analysis phase EvaluationResult to `Sky…
Browse files Browse the repository at this point in the history 
…frameExecutor`.

PiperOrigin-RevId: 489556709
Change-Id: I3b0adbd3082a0765106e3c31e2d294b5afc565eb
  • Loading branch information
@justinhorvitz
justinhorvitz authored and Copybara-Service committed Nov 18, 2022
1 parent c761fa7 commit 0c4de91
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 78 deletions.
97 changes: 22 additions & 75 deletions src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
View file
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -353,7 +351,7 @@ public SkyframeAnalysisResult configureTargets(
int cpuHeavySkyKeysThreadPoolSize)
throws InterruptedException, ViewCreationFailedException {
enableAnalysis(true);
EvaluationResult<ActionLookupValue> result;
ConfigureTargetsResult result;
try (SilentCloseable c = Profiler.instance().profile("skyframeExecutor.configureTargets")) {
result =
skyframeExecutor.configureTargets(
Expand All @@ -367,49 +365,11 @@ public SkyframeAnalysisResult configureTargets(
enableAnalysis(false);
}

int numOfAspects = 0;
if (!topLevelAspectsKeys.isEmpty()) {
numOfAspects =
topLevelAspectsKeys.size()
* topLevelAspectsKeys.get(0).getTopLevelAspectsClasses().size();
}
Map<AspectKey, ConfiguredAspect> aspects = Maps.newHashMapWithExpectedSize(numOfAspects);
Root singleSourceRoot = skyframeExecutor.getForcedSingleSourceRootIfNoExecrootSymlinkCreation();
NestedSetBuilder<Package> packages =
singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null;
ImmutableList.Builder<AspectKey> 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<AspectKey> aspectKeys = aspectKeysBuilder.build();

Collection<ConfiguredTarget> 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<ConfiguredTarget> cts = result.configuredTargets();
ImmutableMap<AspectKey, ConfiguredAspect> aspects = result.aspects();
ImmutableSet<AspectKey> aspectKeys = aspects.keySet();
PackageRoots packageRoots = result.packageRoots();
EvaluationResult<ActionLookupValue> evaluationResult = result.evaluationResult();

ImmutableMap<ActionAnalysisMetadata, ConflictException> actionConflicts = ImmutableMap.of();
try (SilentCloseable c =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -1165,19 +1125,6 @@ private boolean shouldCheckForConflicts(
return false;
}

/** Returns a map of collected package names to root paths. */
private static ImmutableMap<PackageIdentifier, Root> collectPackageRoots(
Collection<Package> packages) {
// Make a map of the package names to their root paths.
ImmutableMap.Builder<PackageIdentifier, Root> 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;
}
Expand Down
71 changes: 68 additions & 3 deletions src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2195,7 +2199,7 @@ protected abstract void invalidateFilesUnderPathForTestingImpl(

/** Configures a given set of configured targets. */
@CanIgnoreReturnValue
EvaluationResult<ActionLookupValue> configureTargets(
ConfigureTargetsResult configureTargets(
ExtendedEventHandler eventHandler,
ImmutableList<ConfiguredTargetKey> configuredTargetKeys,
ImmutableList<TopLevelAspectsKey> topLevelAspectKeys,
Expand All @@ -2219,7 +2223,68 @@ EvaluationResult<ActionLookupValue> configureTargets(
memoizingEvaluator.evaluate(
analysisPhaseKeys(configuredTargetKeys, topLevelAspectKeys), evaluationContext);
perCommandSyscallCache.noteAnalysisPhaseEnded();
return result;

ImmutableSet.Builder<ConfiguredTarget> configuredTargets = ImmutableSet.builder();
ImmutableMap.Builder<AspectKey, ConfiguredAspect> aspects = ImmutableMap.builder();
Root singleSourceRoot = getForcedSingleSourceRootIfNoExecrootSymlinkCreation();
NestedSetBuilder<Package> 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<ActionLookupValue> evaluationResult();

abstract ImmutableSet<ConfiguredTarget> configuredTargets();

abstract ImmutableMap<AspectKey, ConfiguredAspect> aspects();

abstract PackageRoots packageRoots();
}

/** Returns a map of collected package names to root paths. */
private static ImmutableMap<PackageIdentifier, Root> collectPackageRoots(
NestedSet<Package> packages) {
ImmutableMap.Builder<PackageIdentifier, Root> packageRoots = ImmutableMap.builder();
for (Package pkg : packages.toList()) {
if (pkg.getSourceRoot().isPresent()) {
packageRoots.put(pkg.getPackageIdentifier(), pkg.getSourceRoot().get());
}
}
return packageRoots.buildOrThrow();
}

/**
Expand Down Expand Up @@ -3079,7 +3144,7 @@ protected ExecutionFinishedEvent.Builder createExecutionFinishedEventInternal()
}

final AnalysisTraversalResult getActionLookupValuesInBuild(
List<ConfiguredTargetKey> topLevelCtKeys, ImmutableList<AspectKey> aspectKeys)
List<ConfiguredTargetKey> topLevelCtKeys, ImmutableSet<AspectKey> aspectKeys)
throws InterruptedException {
try (SilentCloseable c =
Profiler.instance().profile("skyframeExecutor.getActionLookupValuesInBuild")) {
Expand Down

0 comments on commit 0c4de91

Please sign in to comment.