Skip to content

Commit

Permalink
Introduce a specialized SkyFunctionEnvironment method for a single …
Browse files Browse the repository at this point in the history
…value.

Delegating to `getOrderedValuesAndExceptions` costs significant overhead.

PiperOrigin-RevId: 468196750
Change-Id: I26cc97b9db485a35f2992f114b088f20ea68349f
  • Loading branch information
justinhorvitz authored and copybara-github committed Aug 17, 2022
1 parent df86322 commit 62d1b0b
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ public final class SkyFunctionEnvironmentForTesting extends AbstractSkyFunctionE
this.skyframeExecutor = skyframeExecutor;
}

@Override
protected ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
throws InterruptedException {
return getOrderedValueOrUntypedExceptions(ImmutableList.of(depKey)).get(0);
}

@Override
protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
Iterable<? extends SkyKey> depKeys) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package com.google.devtools.build.skyframe;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.util.GroupedList;
import java.util.ArrayList;
Expand All @@ -40,11 +39,11 @@ public abstract class AbstractSkyFunctionEnvironment implements SkyFunction.Envi
@Nullable private final GroupedList<SkyKey> temporaryDirectDeps;
@Nullable protected List<ListenableFuture<?>> externalDeps;

public AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) {
protected AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) {
this.temporaryDirectDeps = temporaryDirectDeps;
}

public AbstractSkyFunctionEnvironment() {
protected AbstractSkyFunctionEnvironment() {
this(null);
}

Expand All @@ -53,31 +52,47 @@ public GroupedList<SkyKey> getTemporaryDirectDeps() {
return temporaryDirectDeps;
}

/** Implementations should set {@link #valuesMissing} as necessary. */
/**
* Gets a single value or exception.
*
* <p>Implementations should set {@link #valuesMissing} as necessary.
*/
protected abstract ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
throws InterruptedException;

/**
* Gets a map of values or exceptions.
*
* <p>Implementations should set {@link #valuesMissing} as necessary.
*/
protected abstract Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
Iterable<? extends SkyKey> depKeys) throws InterruptedException;

/** Implementations should set {@link #valuesMissing} as necessary. */
/**
* Gets a list of values or exceptions parallel to the given keys.
*
* <p>Implementations should set {@link #valuesMissing} as necessary.
*/
protected abstract List<ValueOrUntypedException> getOrderedValueOrUntypedExceptions(
Iterable<? extends SkyKey> depKeys) throws InterruptedException;

@Override
@Nullable
public SkyValue getValue(SkyKey depKey) throws InterruptedException {
public final SkyValue getValue(SkyKey depKey) throws InterruptedException {
return getValueOrThrowHelper(depKey, null, null, null, null);
}

@Override
@Nullable
public <E extends Exception> SkyValue getValueOrThrow(SkyKey depKey, Class<E> exceptionClass)
throws E, InterruptedException {
public final <E extends Exception> SkyValue getValueOrThrow(
SkyKey depKey, Class<E> exceptionClass) throws E, InterruptedException {
SkyFunctionException.validateExceptionType(exceptionClass);
return getValueOrThrowHelper(depKey, exceptionClass, null, null, null);
}

@Override
@Nullable
public <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(
public final <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(
SkyKey depKey, Class<E1> exceptionClass1, Class<E2> exceptionClass2)
throws E1, E2, InterruptedException {
SkyFunctionException.validateExceptionType(exceptionClass1);
Expand All @@ -87,7 +102,7 @@ public <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(

@Override
@Nullable
public <E1 extends Exception, E2 extends Exception, E3 extends Exception>
public final <E1 extends Exception, E2 extends Exception, E3 extends Exception>
SkyValue getValueOrThrow(
SkyKey depKey,
Class<E1> exceptionClass1,
Expand All @@ -102,7 +117,8 @@ SkyValue getValueOrThrow(

@Override
@Nullable
public <E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception>
public final <
E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception>
SkyValue getValueOrThrow(
SkyKey depKey,
Class<E1> exceptionClass1,
Expand All @@ -127,28 +143,48 @@ SkyValue getValueOrThrowHelper(
@Nullable Class<E3> exceptionClass3,
@Nullable Class<E4> exceptionClass4)
throws E1, E2, E3, E4, InterruptedException {
SkyframeIterableResult result = getOrderedValuesAndExceptions(ImmutableSet.of(depKey));
return result.nextOrThrow(exceptionClass1, exceptionClass2, exceptionClass3, exceptionClass4);
ValueOrUntypedException voe = getSingleValueOrUntypedException(depKey);
SkyValue value = voe.getValue();
if (value != null) {
return value;
}
Exception e = voe.getException();
if (e != null) {
if (exceptionClass1 != null && exceptionClass1.isInstance(e)) {
throw exceptionClass1.cast(e);
}
if (exceptionClass2 != null && exceptionClass2.isInstance(e)) {
throw exceptionClass2.cast(e);
}
if (exceptionClass3 != null && exceptionClass3.isInstance(e)) {
throw exceptionClass3.cast(e);
}
if (exceptionClass4 != null && exceptionClass4.isInstance(e)) {
throw exceptionClass4.cast(e);
}
}
valuesMissing = true;
return null;
}

@Override
public SkyframeLookupResult getValuesAndExceptions(Iterable<? extends SkyKey> depKeys)
public final SkyframeLookupResult getValuesAndExceptions(Iterable<? extends SkyKey> depKeys)
throws InterruptedException {
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
return new SkyframeLookupResult(() -> valuesMissing = true, valuesOrExceptions::get);
}

@Override
public SkyframeIterableResult getOrderedValuesAndExceptions(Iterable<? extends SkyKey> depKeys)
throws InterruptedException {
public final SkyframeIterableResult getOrderedValuesAndExceptions(
Iterable<? extends SkyKey> depKeys) throws InterruptedException {
List<ValueOrUntypedException> valuesOrExceptions = getOrderedValueOrUntypedExceptions(depKeys);
Iterator<ValueOrUntypedException> valuesOrExceptionsi = valuesOrExceptions.iterator();
return new SkyframeIterableResult(() -> valuesMissing = true, valuesOrExceptionsi);
}

@Override
public boolean valuesMissing() {
return valuesMissing || (externalDeps != null);
public final boolean valuesMissing() {
return valuesMissing || externalDeps != null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,30 @@ private static SkyValue getValueOrNullMarker(@Nullable NodeEntry nodeEntry)
return valueMaybeWithMetadata;
}

@Override
protected ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
throws InterruptedException {
checkActive();
checkState(
!depKey.equals(ErrorTransienceValue.KEY),
"Error transience key cannot be in requested deps of %s",
skyKey);
SkyValue depValue = maybeGetValueFromErrorOrDeps(depKey);
if (depValue == null) {
NodeEntry depEntry = evaluatorContext.getGraph().get(skyKey, Reason.DEP_REQUESTED, depKey);
depValue = getValueOrNullMarker(depEntry);
newlyRequestedDepsValues.put(depKey, depValue);
if (depValue != NULL_MARKER) {
maybeUpdateMaxTransitiveSourceVersion(depEntry);
}
}
if (!previouslyRequestedDepsValues.containsKey(depKey)) {
newlyRequestedDeps.add(depKey);
}
processDepValue(depKey, depValue);
return transformToValueOrUntypedException(depValue);
}

@Override
protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
Iterable<? extends SkyKey> depKeys) throws InterruptedException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,12 @@ private BlockingSkyFunctionEnvironment(
this.eventHandler = eventHandler;
}

@Override
protected ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
throws InterruptedException {
return getOrderedValueOrUntypedExceptions(ImmutableList.of(depKey)).get(0);
}

@Override
protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
Iterable<? extends SkyKey> depKeys) {
Expand Down

0 comments on commit 62d1b0b

Please sign in to comment.