Mojo EDK: Circulate MojoArmWatcher outputs to avoid starvation

mojo/edk/system/watcher_dispatcher.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "mojo/edk/system/watcher_dispatcher.h"
 
 #include <limits>
 #include <map>
 
 #include "base/macros.h"
@@ skipping 177 matching lines @@
 
   if (ready_watches_.empty()) {
     // Fast path: No watches are ready to notify, so we're done.
     armed_ = true;
     return MOJO_RESULT_OK;
   }
 
   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;
 
       *(ready_contexts++) = watch->context();
       *(ready_results++) = watch->last_known_result();
       *(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;
 }
 
 WatcherDispatcher::~WatcherDispatcher() {}
 
 }  // namespace edk
 }  // namespace mojo