Move NestedSet expansion logic.
This additionally alleviates the need for the ExpansionWithFutureCallbacks interface.
PiperOrigin-RevId: 300677621
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
index 49c9be4..8f82698 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
@@ -293,6 +293,11 @@
return !(children instanceof Object[] || children instanceof ListenableFuture);
}
+ /** Returns true if this set depends on data from storage. */
+ public boolean isFromStorage() {
+ return children instanceof ListenableFuture;
+ }
+
/** Returns the single element; only call this if {@link #isSingleton} returns true. */
public E getSingleton() {
Preconditions.checkState(isSingleton());
@@ -303,7 +308,7 @@
* Returns an immutable list of all unique elements of the this set, similar to {@link #toList},
* but will propagate an {@code InterruptedException} if one is thrown.
*/
- ImmutableList<E> toListInterruptibly() throws InterruptedException {
+ public ImmutableList<E> toListInterruptibly() throws InterruptedException {
return actualChildrenToList(getChildrenInterruptibly());
}
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetExpander.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetExpander.java
index 9e17577..fe7473f 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetExpander.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSetExpander.java
@@ -13,58 +13,19 @@
// limitations under the License.
package com.google.devtools.build.lib.collect.nestedset;
-import com.google.common.base.Stopwatch;
import com.google.common.collect.ImmutableList;
-import com.google.common.util.concurrent.ListenableFuture;
-import java.time.Duration;
/**
- * Helper class to expand {@link NestedSet} instances which allows to plug in an extra callbacks for
- * expansions waiting for a {@link ListenableFuture}.
+ * Helper class to expand {@link NestedSet} instances.
+ *
+ * <p>Implementations besides {@link NO_CALLBACKS} may wish to implement callbacks or timeouts for
+ * dealing with expansions of sets from storage.
*/
-public class NestedSetExpander {
- public static final NestedSetExpander NO_CALLBACKS =
- new NestedSetExpander(
- new ExpansionWithFutureCallbacks() {
- @Override
- public void onSuccessfulExpansion(Duration time, int size) {}
+public interface NestedSetExpander {
- @Override
- public void onInterruptedExpansion(Duration timeUntilInterrupted) {}
- });
+ <T> ImmutableList<? extends T> toListInterruptibly(NestedSet<? extends T> nestedSet)
+ throws InterruptedException;
- private final ExpansionWithFutureCallbacks expansionWithFutureCallbacks;
-
- /** Callbacks invoked if we expand a {@link NestedSet} backed by a {@link ListenableFuture}. */
- public interface ExpansionWithFutureCallbacks {
- void onSuccessfulExpansion(Duration time, int size);
-
- void onInterruptedExpansion(Duration timeUntilInterrupted);
- }
-
- public NestedSetExpander(ExpansionWithFutureCallbacks expansionWithFutureCallbacks) {
- this.expansionWithFutureCallbacks = expansionWithFutureCallbacks;
- }
-
- /**
- * Returns an immutable list of all unique elements of the the provided set, similar to {@link
- * NestedSet#toList()}, but will propagate an {@code InterruptedException} if one is thrown.
- */
- public final <T> ImmutableList<? extends T> toListInterruptibly(NestedSet<? extends T> nestedSet)
- throws InterruptedException {
- if (!(nestedSet.rawChildren() instanceof ListenableFuture)) {
- return nestedSet.toListInterruptibly();
- }
-
- Stopwatch stopwatch = Stopwatch.createStarted();
- ImmutableList<? extends T> result;
- try {
- result = nestedSet.toListInterruptibly();
- } catch (InterruptedException e) {
- expansionWithFutureCallbacks.onInterruptedExpansion(stopwatch.elapsed());
- throw e;
- }
- expansionWithFutureCallbacks.onSuccessfulExpansion(stopwatch.elapsed(), result.size());
- return result;
- }
+ /** Simply delegates to {@link NestedSet#toListInterruptibly} without doing anything special. */
+ NestedSetExpander DEFAULT = NestedSet::toListInterruptibly;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 7ced0f9..1188368 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -192,7 +192,7 @@
new ConfiguredTargetProgressReceiver(),
/*nonexistentFileReceiver=*/ null,
managedDirectoriesKnowledge,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories);
this.customDirtinessCheckers = customDirtinessCheckers;
this.managedDirectoriesKnowledge = managedDirectoriesKnowledge;
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java
index bf81f3d..1715e15b 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ExecutableSymlinkActionTest.java
@@ -73,7 +73,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index e59af3c..b440f10 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -167,7 +167,7 @@
: ActionInputHelper.actionGraphArtifactExpander(actionGraph),
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
public static ActionExecutionContext createContext(ExtendedEventHandler eventHandler) {
@@ -186,7 +186,7 @@
createDummyArtifactExpander(),
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
public static ActionExecutionContext createContextForInputDiscovery(
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java
index ed3549b..912d858 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/FileWriteActionTestCase.java
@@ -78,7 +78,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
protected void checkNoInputsByDefault() {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
index 7abd5c3..00aaaa2 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/ParamFileWriteActionTest.java
@@ -217,7 +217,7 @@
artifactExpander,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
private enum KeyAttributes {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
index 57559c5..c8389b1 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/SymlinkActionTest.java
@@ -98,7 +98,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS));
+ NestedSetExpander.DEFAULT));
assertThat(actionResult.spawnResults()).isEmpty();
assertThat(output.isSymbolicLink()).isTrue();
assertThat(output.resolveSymbolicLinks()).isEqualTo(input);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java
index 6208962..c00cf5f 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionActionTest.java
@@ -204,7 +204,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
private void executeTemplateExpansion(String expected) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 9531202..a6a685d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -2290,7 +2290,7 @@
artifactExpander,
/*actionFileSystem=*/ null,
/*skyframeDepsResult*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetImplTest.java b/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetImplTest.java
index ab91335..0752f9e 100644
--- a/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetImplTest.java
+++ b/src/test/java/com/google/devtools/build/lib/collect/nestedset/NestedSetImplTest.java
@@ -311,4 +311,17 @@
future.set(new Object[] {"a", "b"});
assertThat(result.get()).containsExactly("a", "b");
}
+
+ @Test
+ public void isFromStorage_true() {
+ NestedSet<?> deserializingNestedSet =
+ NestedSet.withFuture(Order.STABLE_ORDER, SettableFuture.create());
+ assertThat(deserializingNestedSet.isFromStorage()).isTrue();
+ }
+
+ @Test
+ public void isFromStorage_false() {
+ NestedSet<?> inMemoryNestedSet = NestedSetBuilder.create(Order.STABLE_ORDER, "a", "b");
+ assertThat(inMemoryNestedSet.isFromStorage()).isFalse();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
index 49f6d9c..8014026 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/StandaloneTestStrategyTest.java
@@ -109,7 +109,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
this.spawnActionContext = spawnActionContext;
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
index 9d9e1af2..ae4d1d6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
@@ -132,7 +132,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
index 01f355f..71d0b39 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LtoBackendActionTest.java
@@ -99,7 +99,7 @@
/*artifactExpander=*/ null,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
index 8a165a4..e592359 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
@@ -804,7 +804,7 @@
DUMMY_ARTIFACT_EXPANDER,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream();
ByteArrayOutputStream umbrellaHeaderStream = new ByteArrayOutputStream();
moduleMapAction.newDeterministicWriter(dummyActionExecutionContext)
@@ -858,7 +858,7 @@
DUMMY_ARTIFACT_EXPANDER,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
ByteArrayOutputStream moduleMapStream = new ByteArrayOutputStream();
ByteArrayOutputStream umbrellaHeaderStream = new ByteArrayOutputStream();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index b9f28a1..571dcdcb 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -221,7 +221,7 @@
new AtomicReference<>(statusReporter),
/*sourceRootSupplier=*/ () -> ImmutableList.of(),
/*sourceArtifactFactory=*/ unused -> null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
Path actionOutputBase = scratch.dir("/usr/local/google/_blaze_jrluser/FAKEMD5/action_out/");
skyframeActionExecutor.setActionLogBufferPathGenerator(
diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
index 79de9aa..30ac45c 100644
--- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java
@@ -209,7 +209,7 @@
SIMPLE_ARTIFACT_EXPANDER,
/*actionFileSystem=*/ null,
/*skyframeDepsResult=*/ null,
- NestedSetExpander.NO_CALLBACKS);
+ NestedSetExpander.DEFAULT);
}
@Test
