Mojo EDK: Circulate MojoArmWatcher outputs to avoid starvation

mojo/edk/system/watcher_dispatcher.cc

Index: mojo/edk/system/watcher_dispatcher.cc
diff --git a/mojo/edk/system/watcher_dispatcher.cc b/mojo/edk/system/watcher_dispatcher.cc
index e4cc875580ad3f82322cf6559dc3f4ac677d1e71..a5bbfe175909ff5d6b54488812807d014ce86440 100644
--- a/mojo/edk/system/watcher_dispatcher.cc
+++ b/mojo/edk/system/watcher_dispatcher.cc
@@ -195,11 +195,36 @@ MojoResult WatcherDispatcher::Arm(
   if (num_ready_contexts) {
     uint32_t max_ready_contexts = *num_ready_contexts;
     *num_ready_contexts = 0;
-    for (auto& entry : watched_handles_) {
-      if (max_ready_contexts == *num_ready_contexts)
-        break;
 
-      const Watch* watch = entry.second.get();
+    WatchSet::const_iterator next_ready_iter = ready_watches_.begin();
+    if (last_watch_to_block_arming_) {
+      // Find the next watch to notify in simple round-robin order on the
+      // |ready_watches_| map, wrapping around to the beginning if necessary.
+      next_ready_iter = ready_watches_.find(last_watch_to_block_arming_);
+      if (next_ready_iter != ready_watches_.end())
+        ++next_ready_iter;
+      if (next_ready_iter == ready_watches_.end())
+        next_ready_iter = ready_watches_.begin();
+    }
+
+    // Tracks the last watch whose state has been stored in the output buffers.
+    WatchSet::const_iterator last_ready_iter = ready_watches_.end();
+
+    // Tracks of the first Watch we're returning here so we know when we're done
+    // iterating when the caller has given us more space than needed.
+    const WatchSet::const_iterator first_ready_iter = next_ready_iter;
+    bool exhausted = false;
+
+    while (max_ready_contexts > *num_ready_contexts && !exhausted) {
+      const Watch* const watch = *next_ready_iter;
+
+      last_ready_iter = next_ready_iter;
+      ++next_ready_iter;
+      if (next_ready_iter == ready_watches_.end())
+        next_ready_iter = ready_watches_.begin();
+      if (next_ready_iter == first_ready_iter)
+        exhausted = true;
+
       if (!watch->ready())

[yzshen1, 2017/03/16 18:59:01]

[A question to help my own understanding]
Why do we need this check? Because we don't guarantee the user that when the
notification arrives, the signals are still accurate, is it okay to deliver the
state without checking?

If that works, this code could be simplified by setting the correct
max_ready_contexts and removing first_ready_iter/exhausted, I think.

[Ken Rockot(use gerrit already), 2017/03/16 19:21:11]

Very good point! This test was only here because the iteration was happening
over all handles. It is guaranteed that the set of ready_watches_ is equal to
the set of watched_handles_ values for which ready() is true, so this is now
unnecessary. This simplifies things quite a bit.

         continue;
 
@@ -208,6 +233,9 @@ MojoResult WatcherDispatcher::Arm(
       *(ready_signals_states++) = watch->last_known_signals_state();
       ++(*num_ready_contexts);
     }
+
+    if (last_ready_iter != ready_watches_.end())
+      last_watch_to_block_arming_ = *last_ready_iter;
   }
 
   return MOJO_RESULT_FAILED_PRECONDITION;