diff options
author | shreyax <shreyax@google.com> | 2018-02-16 10:50:55 -0800 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-02-16 10:52:23 -0800 |
commit | cce164aed44aba1de244f0d764cd33a5cc6980b2 (patch) | |
tree | 8c62529fdb350eba4cd4bfd930670866e9fade48 /src/main/java/com | |
parent | 68846fe8641a41cd5d522e2deac0d99180085e31 (diff) |
Re-use previously computed deps for TransitiveBaseTraversalFunction#compute if we have them instead of re-computing them each time on a skyframe restart.
PiperOrigin-RevId: 186017079
Diffstat (limited to 'src/main/java/com')
4 files changed, 98 insertions, 10 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java index 5b6e9ed59b..bc6b13b3cf 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableSet; @@ -32,6 +33,7 @@ import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; @@ -61,7 +63,6 @@ import javax.annotation.Nullable; * return. */ abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements SkyFunction { - /** * Returns a {@link SkyKey} corresponding to the traversal of a target specified by {@code label} * and its transitive dependencies. @@ -127,9 +128,9 @@ abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements Sky TargetAndErrorIfAny targetAndErrorIfAny = (TargetAndErrorIfAny) loadTargetResults; TProcessedTargets processedTargets = processTarget(label, targetAndErrorIfAny); - // Process deps from attributes. - Collection<SkyKey> labelDepKeys = - Collections2.transform(getLabelDeps(targetAndErrorIfAny.getTarget()), this::getKey); + // Process deps from attributes. It is essential that the last getValue(s) call we made to + // skyframe for building this node was for the corresponding PackageValue. + Collection<SkyKey> labelDepKeys = getLabelDepKeys(env, targetAndErrorIfAny); Map<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> depMap = env.getValuesOrThrow(labelDepKeys, NoSuchPackageException.class, @@ -139,9 +140,10 @@ abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements Sky return null; } - // Process deps from aspects. + // Process deps from attributes. It is essential that the second-to-last getValue(s) call we + // made to skyframe for building this node was for the corresponding PackageValue. Iterable<SkyKey> labelAspectKeys = - getStrictLabelAspectKeys(targetAndErrorIfAny.getTarget(), depMap, env); + getStrictLabelAspectDepKeys(env, depMap, targetAndErrorIfAny); Set<Entry<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>>> labelAspectEntries = env.getValuesOrThrow(labelAspectKeys, NoSuchPackageException.class, NoSuchTargetException.class).entrySet(); @@ -153,6 +155,60 @@ abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements Sky return computeSkyValue(targetAndErrorIfAny, processedTargets); } + private Collection<SkyKey> getLabelDepKeys( + SkyFunction.Environment env, TargetAndErrorIfAny targetAndErrorIfAny) + throws InterruptedException { + // As a performance optimization we may already know the deps we are about to request from + // last time #compute was called. By requesting these from the environment, we can avoid + // repeating the label visitation step. For TransitiveBaseTraversalFunction#compute, the label + // deps dependency group is requested immediately after the package. + Collection<SkyKey> oldDepKeys = getDepsAfterLastPackageDep(env, /*offset=*/ 1); + return oldDepKeys == null + ? Collections2.transform(getLabelDeps(targetAndErrorIfAny.getTarget()), this::getKey) + : oldDepKeys; + } + + private Iterable<SkyKey> getStrictLabelAspectDepKeys( + SkyFunction.Environment env, + Map<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> depMap, + TargetAndErrorIfAny targetAndErrorIfAny) + throws InterruptedException { + // As a performance optimization we may already know the deps we are about to request from + // last time #compute was called. By requesting these from the environment, we can avoid + // repeating the label visitation step. For TransitiveBaseTraversalFunction#compute, the label + // aspect deps dependency group is requested two groups after the package. + Collection<SkyKey> oldAspectDepKeys = getDepsAfterLastPackageDep(env, /*offset=*/ 2); + return oldAspectDepKeys == null + ? getStrictLabelAspectKeys(targetAndErrorIfAny.getTarget(), depMap, env) + : oldAspectDepKeys; + } + + @Nullable + private static Collection<SkyKey> getDepsAfterLastPackageDep( + SkyFunction.Environment env, int offset) { + GroupedList<SkyKey> temporaryDirectDeps = env.getTemporaryDirectDeps(); + if (temporaryDirectDeps == null) { + return null; + } + int lastPackageDepIndex = getLastPackageValueIndex(temporaryDirectDeps); + if (lastPackageDepIndex == -1 + || temporaryDirectDeps.listSize() <= lastPackageDepIndex + offset) { + return null; + } + return temporaryDirectDeps.get(lastPackageDepIndex + offset); + } + + private static int getLastPackageValueIndex(GroupedList<SkyKey> directDeps) { + int directDepsNumGroups = directDeps.listSize(); + for (int i = directDepsNumGroups - 1; i >= 0; i--) { + List<SkyKey> depGroup = directDeps.get(i); + if (depGroup.size() == 1 && depGroup.get(0).functionName().equals(SkyFunctions.PACKAGE)) { + return i; + } + } + return -1; + } + @Override public String extractTag(SkyKey skyKey) { return Label.print(argumentFromKey(skyKey)); @@ -269,14 +325,18 @@ abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements Sky Target getTarget(); } - private static class TargetAndErrorIfAnyImpl implements TargetAndErrorIfAny, LoadTargetResults { + @VisibleForTesting + static class TargetAndErrorIfAnyImpl implements TargetAndErrorIfAny, LoadTargetResults { private final boolean packageLoadedSuccessfully; @Nullable private final NoSuchTargetException errorLoadingTarget; private final Target target; - private TargetAndErrorIfAnyImpl(boolean packageLoadedSuccessfully, - @Nullable NoSuchTargetException errorLoadingTarget, Target target) { + @VisibleForTesting + TargetAndErrorIfAnyImpl( + boolean packageLoadedSuccessfully, + @Nullable NoSuchTargetException errorLoadingTarget, + Target target) { this.packageLoadedSuccessfully = packageLoadedSuccessfully; this.errorLoadingTarget = errorLoadingTarget; this.target = target; @@ -304,7 +364,7 @@ abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements Sky } } - private LoadTargetResults loadTarget(Environment env, Label label) + protected LoadTargetResults loadTarget(Environment env, Label label) throws NoSuchTargetException, NoSuchPackageException, InterruptedException { SkyKey packageKey = PackageValue.key(label.getPackageIdentifier()); SkyKey targetKey = TargetMarkerValue.key(label); diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java index 7c5deabdd8..2b0984df3a 100644 --- a/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java @@ -17,6 +17,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.devtools.build.lib.util.GroupedList; import com.google.devtools.build.skyframe.ValueOrExceptionUtils.BottomException; import java.util.Collections; import java.util.Map; @@ -29,12 +30,27 @@ import javax.annotation.Nullable; @VisibleForTesting public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Environment { protected boolean valuesMissing = false; + @Nullable private final GroupedList<SkyKey> temporaryDirectDeps; + private <E extends Exception> ValueOrException<E> getValueOrException( SkyKey depKey, Class<E> exceptionClass) throws InterruptedException { return ValueOrExceptionUtils.downconvert( getValueOrException(depKey, exceptionClass, BottomException.class), exceptionClass); } + public AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) { + this.temporaryDirectDeps = temporaryDirectDeps; + } + + public AbstractSkyFunctionEnvironment() { + this(null); + } + + @Override + public GroupedList<SkyKey> getTemporaryDirectDeps() { + return temporaryDirectDeps; + } + private <E1 extends Exception, E2 extends Exception> ValueOrException2<E1, E2> getValueOrException( SkyKey depKey, Class<E1> exceptionClass1, Class<E2> exceptionClass2) diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java index 66e9408d57..9b7c29d2e2 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java @@ -16,6 +16,7 @@ package com.google.devtools.build.skyframe; import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.ExtendedEventHandler; +import com.google.devtools.build.lib.util.GroupedList; import java.util.Map; import javax.annotation.Nullable; @@ -273,6 +274,16 @@ public interface SkyFunction { */ ExtendedEventHandler getListener(); + /** + * A live view of deps known to have already been requested either through an earlier call to + * {@link SkyFunction#compute} or inferred during change pruning. Should return {@code null} if + * unknown. + */ + @Nullable + default GroupedList<SkyKey> getTemporaryDirectDeps() { + return null; + } + /** Returns whether we are currently in error bubbling. */ @VisibleForTesting boolean inErrorBubblingForTesting(); diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java index eb25426a4f..b9830dfebb 100644 --- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java +++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java @@ -129,6 +129,7 @@ class SkyFunctionEnvironment extends AbstractSkyFunctionEnvironment { Set<SkyKey> oldDeps, ParallelEvaluatorContext evaluatorContext) throws InterruptedException { + super(directDeps); this.skyKey = skyKey; this.oldDeps = oldDeps; this.evaluatorContext = evaluatorContext; |