Avoid busy waiting in SchedulerImpl.runRepeatingTasks
If all task are finished in the queue, the code was doing
busy waiting. Added detection for finished task, thus returning
as soon as possible.
Fixes issue 7307.
Change-Id: I288975242600fe5fc13b9bcb356f9807894bb2cb
Review-Link: https://gwt-review.googlesource.com/#/c/1750/
Review by: goktug@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11487 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java b/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java
index fd76da9..14d4381 100644
--- a/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java
+++ b/user/src/com/google/gwt/core/client/impl/SchedulerImpl.java
@@ -149,55 +149,7 @@
return queue;
}
- /**
- * Execute a list of Tasks that hold RepeatingCommands.
- *
- * @return A replacement array that is possibly a shorter copy of
- * <code>tasks</code>
- */
- private static JsArray<Task> runRepeatingTasks(JsArray<Task> tasks) {
- assert tasks != null : "tasks";
- int length = tasks.length();
- if (length == 0) {
- return null;
- }
-
- boolean canceledSomeTasks = false;
- double start = Duration.currentTimeMillis();
-
- while (Duration.currentTimeMillis() - start < TIME_SLICE) {
- for (int i = 0; i < length; i++) {
- assert tasks.length() == length : "Working array length changed "
- + tasks.length() + " != " + length;
- Task t = tasks.get(i);
- if (t == null) {
- continue;
- }
-
- assert t.isRepeating() : "Found a non-repeating Task";
-
- if (!t.executeRepeating()) {
- tasks.set(i, null);
- canceledSomeTasks = true;
- }
- }
- }
-
- if (canceledSomeTasks) {
- JsArray<Task> newTasks = createQueue();
- // Remove tombstones
- for (int i = 0; i < length; i++) {
- if (tasks.get(i) != null) {
- newTasks.push(tasks.get(i));
- }
- }
- assert newTasks.length() < length;
- return newTasks.length() == 0 ? null : newTasks;
- } else {
- return tasks;
- }
- }
/**
* Execute a list of Tasks that hold both ScheduledCommands and
@@ -374,6 +326,13 @@
}
/**
+ * there for testing
+ */
+ Duration createDuration() {
+ return new Duration();
+ }
+
+ /**
* Called by Flusher.
*/
void flushPostEventPumpCommands() {
@@ -412,4 +371,59 @@
scheduleFixedDelayImpl(rescue, RESCUE_DELAY);
}
}
+
+ /**
+ * Execute a list of Tasks that hold RepeatingCommands.
+ *
+ * @return A replacement array that is possibly a shorter copy of <code>tasks</code>
+ */
+ private JsArray<Task> runRepeatingTasks(JsArray<Task> tasks) {
+ assert tasks != null : "tasks";
+
+ int length = tasks.length();
+ if (length == 0) {
+ return null;
+ }
+
+ boolean canceledSomeTasks = false;
+
+ Duration duration = createDuration();
+ while (duration.elapsedMillis() < TIME_SLICE) {
+ boolean executedSomeTask = false;
+ for (int i = 0; i < length; i++) {
+ assert tasks.length() == length : "Working array length changed " + tasks.length() + " != "
+ + length;
+ Task t = tasks.get(i);
+ if (t == null) {
+ continue;
+ }
+ executedSomeTask = true;
+
+ assert t.isRepeating() : "Found a non-repeating Task";
+
+ if (!t.executeRepeating()) {
+ tasks.set(i, null);
+ canceledSomeTasks = true;
+ }
+ }
+ if (!executedSomeTask) {
+ // no work left to do, break to avoid busy waiting until TIME_SLICE is reached
+ break;
+ }
+ }
+
+ if (canceledSomeTasks) {
+ JsArray<Task> newTasks = createQueue();
+ // Remove tombstones
+ for (int i = 0; i < length; i++) {
+ if (tasks.get(i) != null) {
+ newTasks.push(tasks.get(i));
+ }
+ }
+ assert newTasks.length() < length;
+ return newTasks.length() == 0 ? null : newTasks;
+ } else {
+ return tasks;
+ }
+ }
}
diff --git a/user/test/com/google/gwt/core/client/impl/SchedulerImplTest.java b/user/test/com/google/gwt/core/client/impl/SchedulerImplTest.java
index 47f70ad..86a4abd 100644
--- a/user/test/com/google/gwt/core/client/impl/SchedulerImplTest.java
+++ b/user/test/com/google/gwt/core/client/impl/SchedulerImplTest.java
@@ -15,6 +15,7 @@
*/
package com.google.gwt.core.client.impl;
+import com.google.gwt.core.client.Duration;
import com.google.gwt.core.client.JsArray;
import com.google.gwt.core.client.Scheduler.RepeatingCommand;
import com.google.gwt.core.client.Scheduler.ScheduledCommand;
@@ -75,6 +76,22 @@
void schedule(ScheduledCommand cmd);
}
+ private static class RepeatingCommandImpl implements RepeatingCommand {
+ private boolean firstTime = true;
+ private boolean commandRanSecondTime = false;
+
+ @Override
+ public boolean execute() {
+ // Command needs to run for the second time to be executed in runScheduledTasks
+ if (firstTime) {
+ firstTime = false;
+ return true;
+ }
+ commandRanSecondTime = true;
+ return false;
+ }
+ }
+
private static final int TEST_DELAY = 5000;
@Override
@@ -106,6 +123,45 @@
delayTestFinish(TEST_DELAY);
}
+ /**
+ * This test could potentially timeout since loop in {@link SchedulerImpl#runRepeatingTasks} would
+ * run indefinitely since we are mocking Duration to always return zero.
+ *
+ * see for details: https://code.google.com/p/google-web-toolkit/issues/detail?id=7307
+ */
+ public void testEarlyBreakIfAllTaskAreFinished() {
+ final SchedulerImpl impl = new SchedulerImpl() {
+ @Override
+ Duration createDuration() {
+ return new Duration() {
+ @Override
+ public int elapsedMillis() {
+ // never expire
+ return 0;
+ }
+ };
+ }
+ };
+
+ final RepeatingCommandImpl command = new RepeatingCommandImpl();
+
+ impl.scheduleIncremental(command);
+
+ impl.scheduleDeferred(new ScheduledCommand() {
+ @Override
+ public void execute() {
+
+ if (command.commandRanSecondTime) {
+ finishTest();
+ } else {
+ impl.scheduleDeferred(this);
+ }
+ }
+ });
+
+ delayTestFinish(TEST_DELAY);
+ }
+
public void testEntryCommands() {
final SchedulerImpl impl = new SchedulerImpl();
