Skip to content

Commit 62d1b0b

Browse files
justinhorvitzcopybara-github
authored andcommitted
Introduce a specialized SkyFunctionEnvironment method for a single value.
Delegating to `getOrderedValuesAndExceptions` costs significant overhead. PiperOrigin-RevId: 468196750 Change-Id: I26cc97b9db485a35f2992f114b088f20ea68349f
1 parent df86322 commit 62d1b0b

File tree

4 files changed

+90
-18
lines changed

4 files changed

+90
-18
lines changed

src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctionEnvironmentForTesting.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ public final class SkyFunctionEnvironmentForTesting extends AbstractSkyFunctionE
5050
this.skyframeExecutor = skyframeExecutor;
5151
}
5252

53+
@Override
54+
protected ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
55+
throws InterruptedException {
56+
return getOrderedValueOrUntypedExceptions(ImmutableList.of(depKey)).get(0);
57+
}
58+
5359
@Override
5460
protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
5561
Iterable<? extends SkyKey> depKeys) {

src/main/java/com/google/devtools/build/skyframe/AbstractSkyFunctionEnvironment.java

Lines changed: 54 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package com.google.devtools.build.skyframe;
1515

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

43-
public AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) {
42+
protected AbstractSkyFunctionEnvironment(@Nullable GroupedList<SkyKey> temporaryDirectDeps) {
4443
this.temporaryDirectDeps = temporaryDirectDeps;
4544
}
4645

47-
public AbstractSkyFunctionEnvironment() {
46+
protected AbstractSkyFunctionEnvironment() {
4847
this(null);
4948
}
5049

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

56-
/** Implementations should set {@link #valuesMissing} as necessary. */
55+
/**
56+
* Gets a single value or exception.
57+
*
58+
* <p>Implementations should set {@link #valuesMissing} as necessary.
59+
*/
60+
protected abstract ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
61+
throws InterruptedException;
62+
63+
/**
64+
* Gets a map of values or exceptions.
65+
*
66+
* <p>Implementations should set {@link #valuesMissing} as necessary.
67+
*/
5768
protected abstract Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
5869
Iterable<? extends SkyKey> depKeys) throws InterruptedException;
5970

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

6479
@Override
6580
@Nullable
66-
public SkyValue getValue(SkyKey depKey) throws InterruptedException {
81+
public final SkyValue getValue(SkyKey depKey) throws InterruptedException {
6782
return getValueOrThrowHelper(depKey, null, null, null, null);
6883
}
6984

7085
@Override
7186
@Nullable
72-
public <E extends Exception> SkyValue getValueOrThrow(SkyKey depKey, Class<E> exceptionClass)
73-
throws E, InterruptedException {
87+
public final <E extends Exception> SkyValue getValueOrThrow(
88+
SkyKey depKey, Class<E> exceptionClass) throws E, InterruptedException {
7489
SkyFunctionException.validateExceptionType(exceptionClass);
7590
return getValueOrThrowHelper(depKey, exceptionClass, null, null, null);
7691
}
7792

7893
@Override
7994
@Nullable
80-
public <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(
95+
public final <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(
8196
SkyKey depKey, Class<E1> exceptionClass1, Class<E2> exceptionClass2)
8297
throws E1, E2, InterruptedException {
8398
SkyFunctionException.validateExceptionType(exceptionClass1);
@@ -87,7 +102,7 @@ public <E1 extends Exception, E2 extends Exception> SkyValue getValueOrThrow(
87102

88103
@Override
89104
@Nullable
90-
public <E1 extends Exception, E2 extends Exception, E3 extends Exception>
105+
public final <E1 extends Exception, E2 extends Exception, E3 extends Exception>
91106
SkyValue getValueOrThrow(
92107
SkyKey depKey,
93108
Class<E1> exceptionClass1,
@@ -102,7 +117,8 @@ SkyValue getValueOrThrow(
102117

103118
@Override
104119
@Nullable
105-
public <E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception>
120+
public final <
121+
E1 extends Exception, E2 extends Exception, E3 extends Exception, E4 extends Exception>
106122
SkyValue getValueOrThrow(
107123
SkyKey depKey,
108124
Class<E1> exceptionClass1,
@@ -127,28 +143,48 @@ SkyValue getValueOrThrowHelper(
127143
@Nullable Class<E3> exceptionClass3,
128144
@Nullable Class<E4> exceptionClass4)
129145
throws E1, E2, E3, E4, InterruptedException {
130-
SkyframeIterableResult result = getOrderedValuesAndExceptions(ImmutableSet.of(depKey));
131-
return result.nextOrThrow(exceptionClass1, exceptionClass2, exceptionClass3, exceptionClass4);
146+
ValueOrUntypedException voe = getSingleValueOrUntypedException(depKey);
147+
SkyValue value = voe.getValue();
148+
if (value != null) {
149+
return value;
150+
}
151+
Exception e = voe.getException();
152+
if (e != null) {
153+
if (exceptionClass1 != null && exceptionClass1.isInstance(e)) {
154+
throw exceptionClass1.cast(e);
155+
}
156+
if (exceptionClass2 != null && exceptionClass2.isInstance(e)) {
157+
throw exceptionClass2.cast(e);
158+
}
159+
if (exceptionClass3 != null && exceptionClass3.isInstance(e)) {
160+
throw exceptionClass3.cast(e);
161+
}
162+
if (exceptionClass4 != null && exceptionClass4.isInstance(e)) {
163+
throw exceptionClass4.cast(e);
164+
}
165+
}
166+
valuesMissing = true;
167+
return null;
132168
}
133169

134170
@Override
135-
public SkyframeLookupResult getValuesAndExceptions(Iterable<? extends SkyKey> depKeys)
171+
public final SkyframeLookupResult getValuesAndExceptions(Iterable<? extends SkyKey> depKeys)
136172
throws InterruptedException {
137173
Map<SkyKey, ValueOrUntypedException> valuesOrExceptions = getValueOrUntypedExceptions(depKeys);
138174
return new SkyframeLookupResult(() -> valuesMissing = true, valuesOrExceptions::get);
139175
}
140176

141177
@Override
142-
public SkyframeIterableResult getOrderedValuesAndExceptions(Iterable<? extends SkyKey> depKeys)
143-
throws InterruptedException {
178+
public final SkyframeIterableResult getOrderedValuesAndExceptions(
179+
Iterable<? extends SkyKey> depKeys) throws InterruptedException {
144180
List<ValueOrUntypedException> valuesOrExceptions = getOrderedValueOrUntypedExceptions(depKeys);
145181
Iterator<ValueOrUntypedException> valuesOrExceptionsi = valuesOrExceptions.iterator();
146182
return new SkyframeIterableResult(() -> valuesMissing = true, valuesOrExceptionsi);
147183
}
148184

149185
@Override
150-
public boolean valuesMissing() {
151-
return valuesMissing || (externalDeps != null);
186+
public final boolean valuesMissing() {
187+
return valuesMissing || externalDeps != null;
152188
}
153189

154190
@Override

src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,30 @@ private static SkyValue getValueOrNullMarker(@Nullable NodeEntry nodeEntry)
428428
return valueMaybeWithMetadata;
429429
}
430430

431+
@Override
432+
protected ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
433+
throws InterruptedException {
434+
checkActive();
435+
checkState(
436+
!depKey.equals(ErrorTransienceValue.KEY),
437+
"Error transience key cannot be in requested deps of %s",
438+
skyKey);
439+
SkyValue depValue = maybeGetValueFromErrorOrDeps(depKey);
440+
if (depValue == null) {
441+
NodeEntry depEntry = evaluatorContext.getGraph().get(skyKey, Reason.DEP_REQUESTED, depKey);
442+
depValue = getValueOrNullMarker(depEntry);
443+
newlyRequestedDepsValues.put(depKey, depValue);
444+
if (depValue != NULL_MARKER) {
445+
maybeUpdateMaxTransitiveSourceVersion(depEntry);
446+
}
447+
}
448+
if (!previouslyRequestedDepsValues.containsKey(depKey)) {
449+
newlyRequestedDeps.add(depKey);
450+
}
451+
processDepValue(depKey, depValue);
452+
return transformToValueOrUntypedException(depValue);
453+
}
454+
431455
@Override
432456
protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
433457
Iterable<? extends SkyKey> depKeys) throws InterruptedException {

src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,12 @@ private BlockingSkyFunctionEnvironment(
333333
this.eventHandler = eventHandler;
334334
}
335335

336+
@Override
337+
protected ValueOrUntypedException getSingleValueOrUntypedException(SkyKey depKey)
338+
throws InterruptedException {
339+
return getOrderedValueOrUntypedExceptions(ImmutableList.of(depKey)).get(0);
340+
}
341+
336342
@Override
337343
protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
338344
Iterable<? extends SkyKey> depKeys) {

0 commit comments

Comments
 (0)