Skip to content

Commit

Permalink
More migrations off of GetOrderedValuesAndExceptions.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 484304682
Change-Id: Icd0298644639338222035c76d31b211f4ec74716
  • Loading branch information
justinhorvitz authored and copybara-github committed Oct 27, 2022
1 parent 50b87c1 commit 8761fc5
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -1840,16 +1840,15 @@ public CompileCommandLine getCompileCommandLine() {
@Nullable
private static ImmutableMap<Artifact, NestedSet<Artifact>> computeTransitivelyUsedModules(
SkyFunction.Environment env, Set<DerivedArtifact> usedModules) throws InterruptedException {
// Because SkyframeIterableResult.next call does not specify any exceptions where
// SkyframeIterableResult is returned by env.getOrderedValuesAndExceptions, it is
// impossible for input discovery to recover from exceptions thrown by spurious module deps (for
// instance, if a commented-out include references a header file with an error in it). However,
// we generally don't try to recover from errors around spurious includes discovered in the
// current build.
// Because SkyframeLookupResult.get call does not specify any exceptions where
// SkyframeLookupResult is returned by env.getValuesAndExceptions, it is impossible for input
// discovery to recover from exceptions thrown by spurious module deps (for instance, if a
// commented-out include references a header file with an error in it). However, we generally
// don't try to recover from errors around spurious includes discovered in the current build.
// TODO(janakr): Can errors be aggregated here at least?
Collection<SkyKey> skyKeys =
Collections2.transform(usedModules, DerivedArtifact::getGeneratingActionKey);
SkyframeIterableResult actionExecutionValues = env.getOrderedValuesAndExceptions(skyKeys);
SkyframeLookupResult actionExecutionValues = env.getValuesAndExceptions(skyKeys);
if (env.valuesMissing()) {
return null;
}
Expand All @@ -1858,7 +1857,7 @@ private static ImmutableMap<Artifact, NestedSet<Artifact>> computeTransitivelyUs
for (DerivedArtifact module : usedModules) {
Preconditions.checkState(
module.isFileType(CppFileTypes.CPP_MODULE), "Non-module? %s", module);
SkyValue skyValue = actionExecutionValues.next();
SkyValue skyValue = actionExecutionValues.get(module.getGeneratingActionKey());
if (skyValue == null) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.Set;
import java.util.stream.Stream;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -66,12 +66,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
// Avoid silly cycles.
depKeys.remove(skyKey);

SkyframeIterableResult result = env.getOrderedValuesAndExceptions(depKeys);
SkyframeLookupResult result = env.getValuesAndExceptions(depKeys);
if (env.valuesMissing()) {
return null;
}
while (result.hasNext()) {
if (result.next() == null) {
for (SkyKey key : depKeys) {
if (result.get(key) == null) {
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -433,9 +434,9 @@ private static void postAspectAnalyzedEvent(
*/
private static void declareDependenciesAndCheckValues(
Environment env, Iterable<? extends SkyKey> skyKeys) throws InterruptedException {
SkyframeIterableResult result = env.getOrderedValuesAndExceptions(skyKeys);
while (result.hasNext()) {
if (result.next() == null) {
SkyframeLookupResult result = env.getValuesAndExceptions(skyKeys);
for (SkyKey key : skyKeys) {
if (result.get(key) == null) {
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.Map;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -61,20 +61,20 @@ public class BuildInfoCollectionFunction implements SkyFunction {
public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedException {
final BuildInfoKeyAndConfig keyAndConfig = (BuildInfoKeyAndConfig) skyKey.argument();
ImmutableSet<SkyKey> keysToRequest =
ImmutableSet.of(
WorkspaceStatusValue.BUILD_INFO_KEY,
keyAndConfig.getConfigurationKey());
SkyframeIterableResult result = env.getOrderedValuesAndExceptions(keysToRequest);
ImmutableSet.of(WorkspaceStatusValue.BUILD_INFO_KEY, keyAndConfig.getConfigurationKey());
SkyframeLookupResult result = env.getValuesAndExceptions(keysToRequest);
if (env.valuesMissing()) {
return null;
}
WorkspaceStatusValue infoArtifactValue = (WorkspaceStatusValue) result.next();
WorkspaceStatusValue infoArtifactValue =
(WorkspaceStatusValue) result.get(WorkspaceStatusValue.BUILD_INFO_KEY);
if (infoArtifactValue == null) {
BugReport.logUnexpected("Value for: BuildInfoKey was missing, this should never happen");
return null;
}

BuildConfigurationValue config = (BuildConfigurationValue) result.next();
BuildConfigurationValue config =
(BuildConfigurationValue) result.get(keyAndConfig.getConfigurationKey());
if (config == null) {
BugReport.logUnexpected(
"Value for: '%s' was missing, this should never happen",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -1009,11 +1009,11 @@ private static List<BzlLoadValue> computeBzlLoadsWithSkyframe(
Environment env, List<BzlLoadValue.Key> keys, List<Pair<String, Location>> programLoads)
throws BzlLoadFailedException, InterruptedException {
List<BzlLoadValue> bzlLoads = Lists.newArrayListWithExpectedSize(keys.size());
SkyframeIterableResult values = env.getOrderedValuesAndExceptions(keys);
SkyframeLookupResult values = env.getValuesAndExceptions(keys);
// Process loads (and report first error) in source order.
for (int i = 0; i < keys.size(); i++) {
try {
bzlLoads.add((BzlLoadValue) values.nextOrThrow(BzlLoadFailedException.class));
bzlLoads.add((BzlLoadValue) values.getOrThrow(keys.get(i), BzlLoadFailedException.class));
} catch (BzlLoadFailedException ex) {
throw BzlLoadFailedException.whileLoadingDep(programLoads.get(i).second, ex);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
Expand All @@ -71,19 +71,6 @@ public final class CompletionFunction<
interface Completor<
ValueT, ResultT extends SkyValue, KeyT extends TopLevelActionLookupKey, FailureT> {

/**
* Returns the options which determine the artifacts to build for the top-level targets.
*
* <p>For the Top level targets we made a conscious decision to include the
* TopLevelArtifactContext within the SkyKey as an argument to the CompletionFunction rather
* than a separate SkyKey. As a result we do have <num top level targets> extra SkyKeys for
* every unique TopLevelArtifactContexts used over the lifetime of Blaze. This is a minor
* tradeoff, since it significantly improves null build times when we're switching the
* TopLevelArtifactContexts frequently (common for IDEs), by reusing existing SkyKeys from
* earlier runs, instead of causing an eager invalidation were the TopLevelArtifactContext
* modeled as a separate SkyKey.
*/

/** Creates an event reporting an absent input artifact. */
Event getRootCauseError(ValueT value, KeyT key, LabelCause rootCause, Environment env)
throws InterruptedException;
Expand Down Expand Up @@ -161,8 +148,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
ArtifactsToBuild artifactsToBuild = valueAndArtifactsToBuild.second;

ImmutableList<Artifact> allArtifacts = artifactsToBuild.getAllArtifacts().toList();
SkyframeIterableResult inputDeps =
env.getOrderedValuesAndExceptions(Artifact.keys(allArtifacts));
SkyframeLookupResult inputDeps = env.getValuesAndExceptions(Artifact.keys(allArtifacts));

boolean allArtifactsAreImportant = artifactsToBuild.areAllOutputGroupsImportant();

Expand Down Expand Up @@ -197,7 +183,8 @@ public SkyValue compute(SkyKey skyKey, Environment env)
for (Artifact input : allArtifacts) {
try {
SkyValue artifactValue =
inputDeps.nextOrThrow(ActionExecutionException.class, SourceArtifactException.class);
inputDeps.getOrThrow(
Artifact.key(input), ActionExecutionException.class, SourceArtifactException.class);
if (artifactValue != null) {
if (artifactValue instanceof MissingArtifactValue) {
handleSourceFileError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyframeIterableResult;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -138,14 +138,16 @@ public ProcessPackageDirectoryResult getPackageExistenceAndSubdirDeps(

SkyKey pkgLookupKey = PackageLookupValue.key(packageId);
SkyKey dirListingKey = DirectoryListingValue.key(rootedPath);
SkyframeIterableResult pkgLookupAndDirectoryListingDeps =
env.getOrderedValuesAndExceptions(ImmutableList.of(pkgLookupKey, dirListingKey));
SkyframeLookupResult pkgLookupAndDirectoryListingDeps =
env.getValuesAndExceptions(ImmutableList.of(pkgLookupKey, dirListingKey));
PackageLookupValue pkgLookupValue;
try {
pkgLookupValue =
(PackageLookupValue)
pkgLookupAndDirectoryListingDeps.nextOrThrow(
NoSuchPackageException.class, InconsistentFilesystemException.class);
pkgLookupAndDirectoryListingDeps.getOrThrow(
pkgLookupKey,
NoSuchPackageException.class,
InconsistentFilesystemException.class);
} catch (NoSuchPackageException e) {
return reportErrorAndReturn("Failed to load package", e, rootRelativePath, env.getListener());
} catch (InconsistentFilesystemException e) {
Expand All @@ -154,7 +156,8 @@ public ProcessPackageDirectoryResult getPackageExistenceAndSubdirDeps(
DirectoryListingValue dirListingValue;
try {
dirListingValue =
(DirectoryListingValue) pkgLookupAndDirectoryListingDeps.nextOrThrow(IOException.class);
(DirectoryListingValue)
pkgLookupAndDirectoryListingDeps.getOrThrow(dirListingKey, IOException.class);
} catch (FileSymlinkException e) {
// DirectoryListingFunction only throws FileSymlinkCycleException when FileFunction throws it,
// but FileFunction was evaluated for rootedPath above, and didn't throw there. It shouldn't
Expand Down

0 comments on commit 8761fc5

Please sign in to comment.