Bug 1677509 - Use GetQueuedCompletionStatusEx in win message pump r=handyman

This is almost certainly a very small optimization, and likely won't address
the frequency of hangs from bug 1677509. However, it still should be an
improvement, and will work on anything Vista or later.

We observed as part of trying to diagnose the somewhat mysterious bug 1677509
that sometimes multiple messages would be queued, and yet the IPC I/O thread
would go idle in between servicing them in GetQueuedCompletionStatusEx. Given
that we send frequent sync ipc messages for mouse move events, it seems prudent
to be able to service multiple at a time.

Differential Revision: https://phabricator.services.mozilla.com/D114287

ipc/chromium/src/base/message_pump_win.cc

@@ -452,45 +452,55 @@ void MessagePumpForIO::WaitForWork() {
 }
 
 bool MessagePumpForIO::WaitForIOCompletion(DWORD timeout, IOHandler* filter) {
-  IOItem item;
-  if (completed_io_.empty() || !MatchCompletedIOItem(filter, &item)) {
-    // We have to ask the system for another IO completion.
-    if (!GetIOItem(timeout, &item)) return false;
-
-    if (ProcessInternalIOItem(item)) return true;
-  }
-
-  if (item.context->handler) {
-    if (filter && item.handler != filter) {
-      // Save this item for later
-      completed_io_.push_back(item);
-    } else {
-      DCHECK(item.context->handler == item.handler);
-      item.handler->OnIOCompleted(item.context, item.bytes_transfered,
-                                  item.error);
-    }
+  IOItemChunk items;
+  if (completed_io_.empty() || !MatchCompletedIOItem(filter, items.values)) {
+    if (!GetIOItems(timeout, &items)) return false;
   } else {
-    // The handler must be gone by now, just cleanup the mess.
-    delete item.context;
+    items.count = 1;
   }
+
+  for (ULONG i = 0; i < items.count; ++i) {
+    IOItem& item = items.values[i];
+    if (ProcessInternalIOItem(item)) {
+      continue;
+    }
+
+    if (item.context->handler) {
+      if (filter && item.handler != filter) {
+        // Save this item for later
+        completed_io_.push_back(item);
+      } else {
+        DCHECK(item.context->handler == item.handler);
+        item.handler->OnIOCompleted(item.context, item.bytes_transfered);
+      }
+    } else {
+      // The handler must be gone by now, just cleanup the mess.
+      delete item.context;
+    }
+  }
+
   return true;
 }
 
 // Asks the OS for another IO completion result.
-bool MessagePumpForIO::GetIOItem(DWORD timeout, IOItem* item) {
-  memset(item, 0, sizeof(*item));
-  ULONG_PTR key = 0;
-  OVERLAPPED* overlapped = NULL;
+bool MessagePumpForIO::GetIOItems(DWORD timeout, IOItemChunk* items) {
+  memset(items, 0, sizeof(*items));
+  OVERLAPPED_ENTRY entries[arraysize(items->values)];
   AUTO_PROFILER_LABEL("MessagePumpForIO::GetIOItem::Wait", IDLE);
-  if (!GetQueuedCompletionStatus(port_.Get(), &item->bytes_transfered, &key,
-                                 &overlapped, timeout)) {
-    if (!overlapped) return false;  // Nothing in the queue.
-    item->error = GetLastError();
-    item->bytes_transfered = 0;
+  if (!GetQueuedCompletionStatusEx(port_.Get(), entries,
+                                   arraysize(items->values), &items->count,
+                                   timeout, FALSE)) {
+    return false;  // Nothing in the queue.
+  }
+
+  for (int i = 0; i < (int)items->count; ++i) {
+    items->values[i].handler =
+        reinterpret_cast<IOHandler*>(entries[i].lpCompletionKey);
+    items->values[i].bytes_transfered = entries[i].dwNumberOfBytesTransferred;
+    items->values[i].context =
+        reinterpret_cast<IOContext*>(entries[i].lpOverlapped);
   }
 
-  item->handler = reinterpret_cast<IOHandler*>(key);
-  item->context = reinterpret_cast<IOContext*>(overlapped);
   return true;
 }
 

ipc/chromium/src/base/message_pump_win.h

@@ -222,8 +222,8 @@ class MessagePumpForIO : public MessagePumpWin {
   //         delete context_;
   //      }
   //     }
-  //     virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered,
-  //                                DWORD error) {
+  //     virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered)
+  //     {
   //         pending_ = false;
   //     }
   //     void DoSomeIo() {
@@ -248,8 +248,8 @@ class MessagePumpForIO : public MessagePumpWin {
   //     // while there are pending IO operations.
   //     ~MyFile() {
   //     }
-  //     virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered,
-  //                                DWORD error) {
+  //     virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered)
+  //     {
   //       ...
   //       delete context;
   //     }
@@ -280,8 +280,7 @@ class MessagePumpForIO : public MessagePumpWin {
     // |context| completes. |error| is the Win32 error code of the IO operation
     // (ERROR_SUCCESS if there was no error). |bytes_transfered| will be zero
     // on error.
-    virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered,
-                               DWORD error) = 0;
+    virtual void OnIOCompleted(IOContext* context, DWORD bytes_transfered) = 0;
   };
 
   // The extended context that should be used as the base structure on every
@@ -326,13 +325,16 @@ class MessagePumpForIO : public MessagePumpWin {
     IOHandler* handler;
     IOContext* context;
     DWORD bytes_transfered;
-    DWORD error;
+  };
+  struct IOItemChunk {
+    IOItem values[8];
+    ULONG count;
   };
 
   virtual void DoRunLoop();
   void WaitForWork();
   bool MatchCompletedIOItem(IOHandler* filter, IOItem* item);
-  bool GetIOItem(DWORD timeout, IOItem* item);
+  bool GetIOItems(DWORD timeout, IOItemChunk* items);
   bool ProcessInternalIOItem(const IOItem& item);
 
   // The completion port associated with this thread.

ipc/chromium/src/chrome/common/ipc_channel_win.cc

@@ -286,7 +286,7 @@ bool Channel::ChannelImpl::Connect() {
     // to true, we indicate to OnIOCompleted that this is the special
     // initialization signal.
     MessageLoopForIO::current()->PostTask(factory_.NewRunnableMethod(
-        &Channel::ChannelImpl::OnIOCompleted, &input_state_.context, 0, 0));
+        &Channel::ChannelImpl::OnIOCompleted, &input_state_.context, 0));
   }
 
   if (!waiting_connect_) ProcessOutgoingMessages(NULL, 0);
@@ -556,7 +556,7 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages(
 }
 
 void Channel::ChannelImpl::OnIOCompleted(MessageLoopForIO::IOContext* context,
-                                         DWORD bytes_transfered, DWORD error) {
+                                         DWORD bytes_transfered) {
   bool ok;
   ASSERT_OWNINGTHREAD(ChannelImpl);
   if (context == &input_state_.context) {

ipc/chromium/src/chrome/common/ipc_channel_win.h

@@ -68,7 +68,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
 
   // MessageLoop::IOHandler implementation.
   virtual void OnIOCompleted(MessageLoopForIO::IOContext* context,
-                             DWORD bytes_transfered, DWORD error);
+                             DWORD bytes_transfered);
 
  private:
   struct State {