Make the action completion watchdog wait instead of waiting for the first
possible action completion to reduce contention. This isn't very pretty, but I
think should be good enough.
RELNOTES: None.
PiperOrigin-RevId: 209111759
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
index 484298c..4a5709d 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
@@ -18,7 +18,6 @@
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata.MiddlemanType;
import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter;
import com.google.devtools.build.lib.actions.ActionLookupData;
-import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.skyframe.ActionExecutionInactivityWatchdog;
import com.google.devtools.build.lib.skyframe.AspectCompletionValue;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
@@ -61,7 +60,6 @@
private final Set<ActionLookupData> completedActions = Sets.newConcurrentHashSet();
private final Set<ActionLookupData> ignoredActions = Sets.newConcurrentHashSet();
- private final Object activityIndicator = new Object();
/** Number of exclusive tests. To be accounted for in progress messages. */
private final int exclusiveTestsCount;
@@ -140,9 +138,6 @@
public void actionCompleted(ActionLookupData actionLookupData) {
if (!ignoredActions.contains(actionLookupData)) {
completedActions.add(actionLookupData);
- synchronized (activityIndicator) {
- activityIndicator.notifyAll();
- }
}
}
@@ -175,28 +170,17 @@
}
@Override
- public int waitForNextCompletion(int timeoutMilliseconds) throws InterruptedException {
- long rest = timeoutMilliseconds;
- synchronized (activityIndicator) {
- int before = completedActions.size();
- long startTime = BlazeClock.instance().currentTimeMillis();
- while (true) {
- activityIndicator.wait(rest);
-
- int completed = completedActions.size() - before;
- long now = 0;
- if (completed > 0
- || (startTime + rest)
- <= (now = BlazeClock.instance().currentTimeMillis())) {
- // Some actions completed, or timeout fully elapsed.
- return completed;
- } else {
- // Spurious Wakeup -- no actions completed and there's still time to wait.
- rest -= now - startTime; // account for elapsed wait time
- startTime = now;
- }
+ public int waitForNextCompletion(int timeoutSeconds) throws InterruptedException {
+ int before = completedActions.size();
+ // Otherwise, wake up once per second to see whether something completed.
+ for (int i = 0; i < timeoutSeconds; i++) {
+ Thread.sleep(1000);
+ int count = completedActions.size() - before;
+ if (count > 0) {
+ return count;
}
}
+ return 0;
}
};
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdog.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdog.java
index 94e12fb..35446dc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdog.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdog.java
@@ -45,7 +45,7 @@
*
* @return the number of actions completed during the wait
*/
- int waitForNextCompletion(int timeoutMilliseconds) throws InterruptedException;
+ int waitForNextCompletion(int timeoutSeconds) throws InterruptedException;
}
/** An object that the watchdog can report inactivity to. */
@@ -145,7 +145,7 @@
// Wait a while for any SkyFunction to finish. The returned number indicates how many
// actions completed during the wait. It's possible that this is more than 1, since
// this thread may not immediately regain control.
- int completedActions = monitor.waitForNextCompletion(waitTime.next() * 1000);
+ int completedActions = monitor.waitForNextCompletion(waitTime.next());
if (!isRunning.get()) {
break;
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdogTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdogTest.java
index ee6f419..986166b 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdogTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ActionExecutionInactivityWatchdogTest.java
@@ -49,7 +49,7 @@
InactivityMonitor monitor =
new InactivityMonitor() {
@Override
- public int waitForNextCompletion(int timeoutMilliseconds) throws InterruptedException {
+ public int waitForNextCompletion(int timeoutSeconds) throws InterruptedException {
// Simulate the following sequence of events (see actionCompletions):
// 1. return in 5s (within timeout), 1 action completed; caller will sleep
// 2. return in 10s (after timeout), 0 action completed; caller will wait
