Skip to content

Commit

Permalink
Replace getValuesOrThrow with getValuesAndExceptions in `Configur…
Browse files Browse the repository at this point in the history
…edTargetFunction` to create less garbage.

Instead of returning `Map`, returning `SkyframeLookupResult` saves memory. And the new method can avoid temporary wrapper objects.

Change checking `ValueOrUntypedException` to checking presence of `packageKey` in a set of `packageKeys` to detect unrequested packages for alias targets.

PiperOrigin-RevId: 433223776
  • Loading branch information
emilyguo1 authored and copybara-github committed Mar 8, 2022
1 parent 6fbb159 commit b59498a
Showing 1 changed file with 29 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.causes.AnalysisFailedCause;
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.causes.LoadingFailedCause;
Expand Down Expand Up @@ -89,8 +90,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.ValueOrException;
import com.google.devtools.build.skyframe.ValueOrUntypedException;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
Expand Down Expand Up @@ -644,8 +644,7 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
.build());
}

Map<SkyKey, ValueOrException<ToolchainException>> values =
env.getValuesOrThrow(toolchainContextKeys.values(), ToolchainException.class);
SkyframeLookupResult values = env.getValuesAndExceptions(toolchainContextKeys.values());

boolean valuesMissing = env.valuesMissing();

Expand All @@ -654,7 +653,16 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
for (Map.Entry<String, ToolchainContextKey> unloadedToolchainContextKey :
toolchainContextKeys.entrySet()) {
UnloadedToolchainContext unloadedToolchainContext =
(UnloadedToolchainContext) values.get(unloadedToolchainContextKey.getValue()).get();
(UnloadedToolchainContext)
values.getOrThrow(unloadedToolchainContextKey.getValue(), ToolchainException.class);
if (valuesMissing != env.valuesMissing()) {
BugReport.sendBugReport(
new IllegalStateException(
"ToolchainContextValue "
+ unloadedToolchainContextKey.getValue()
+ " was missing, this should never happen"));
break;
}
if (!valuesMissing) {
String execGroup = unloadedToolchainContextKey.getKey();
if (execGroup.equals(targetUnloadedToolchainContext)) {
Expand All @@ -666,7 +674,7 @@ public static ComputedToolchainContexts computeUnloadedToolchainContexts(
}

ComputedToolchainContexts result = new ComputedToolchainContexts();
result.toolchainCollection = valuesMissing ? null : toolchainContexts.build();
result.toolchainCollection = env.valuesMissing() ? null : toolchainContexts.build();
result.execGroupCollectionBuilder = execGroupCollectionBuilder;
return result;
}
Expand Down Expand Up @@ -972,13 +980,14 @@ private static Map<SkyKey, ConfiguredTargetAndData> resolveConfiguredTargetDepen
// associated Targets (and therefore associated Packages) don't correspond to their own Labels.
// We don't know the associated Package until we fetch the ConfiguredTarget. Therefore, we have
// to do a potential second pass, in which we fetch all the Packages for AliasConfiguredTargets.
Iterable<SkyKey> depKeys =
Iterables.concat(
Iterables.transform(deps, Dependency::getConfiguredTargetKey),
ImmutableSet<SkyKey> packageKeys =
ImmutableSet.copyOf(
Iterables.transform(
deps, input -> PackageValue.key(input.getLabel().getPackageIdentifier())));
Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> depValuesOrExceptions =
env.getValuesOrThrow(depKeys, ConfiguredValueCreationException.class);
Iterable<SkyKey> depKeys =
Iterables.concat(
Iterables.transform(deps, Dependency::getConfiguredTargetKey), packageKeys);
SkyframeLookupResult depValuesOrExceptions = env.getValuesAndExceptions(depKeys);
Map<SkyKey, ConfiguredTargetAndData> result = Maps.newHashMapWithExpectedSize(deps.size());
Set<SkyKey> aliasPackagesToFetch = new HashSet<>();
List<Dependency> aliasDepsToRedo = new ArrayList<>();
Expand All @@ -989,7 +998,9 @@ private static Map<SkyKey, ConfiguredTargetAndData> resolveConfiguredTargetDepen
SkyKey key = dep.getConfiguredTargetKey();
ConfiguredTargetValue depValue;
try {
depValue = (ConfiguredTargetValue) depValuesOrExceptions.get(key).get();
depValue =
(ConfiguredTargetValue)
depValuesOrExceptions.getOrThrow(key, ConfiguredValueCreationException.class);
} catch (ConfiguredValueCreationException e) {
transitiveRootCauses.addTransitive(e.getRootCauses());
detailedExitCode =
Expand All @@ -1010,18 +1021,17 @@ private static Map<SkyKey, ConfiguredTargetAndData> resolveConfiguredTargetDepen
SkyKey packageKey = PackageValue.key(depLabel.getPackageIdentifier());
PackageValue pkgValue;
if (i == 0) {
ValueOrUntypedException packageResult = depValuesOrExceptions.get(packageKey);
if (packageResult == null) {
if (!packageKeys.contains(packageKey)) {
aliasPackagesToFetch.add(packageKey);
aliasDepsToRedo.add(dep);
continue;
} else {
pkgValue = (PackageValue) packageResult.getUnchecked();
pkgValue = (PackageValue) depValuesOrExceptions.get(packageKey);
if (pkgValue == null) {
// In a race, the getValuesOrThrow call above may have retrieved the package before it
// was done but the configured target after it was done. Since SkyFunctionEnvironment
// may cache absent values, re-requesting it on this evaluation may be useless, just
// treat it as missing.
// In a race, the getValuesAndExceptions call above may have retrieved the package
// before it was done but the configured target after it was done. Since
// SkyFunctionEnvironment may cache absent values, re-requesting it on this evaluation
// may be useless, just treat it as missing.
missedValues = true;
continue;
}
Expand Down

0 comments on commit b59498a

Please sign in to comment.