Bug 43148

Summary: Investigate using an I/O completion port for WorkQueue on Windows
Product: WebKit Reporter: Adam Roben (:aroben) <aroben@webkit.org>
Component: WebKit2Assignee: Nobody <webkit-unassigned@lists.webkit.org>
Status: RESOLVED INVALID    
Severity: Normal CC: andersca@apple.com, fishd@chromium.org, sam@webkit.org
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Bug Depends on: 44185    
Bug Blocks:    

Description From 2010-07-28 14:48:57 PST
I/O completion ports [1] are supposedly tailor-made for a use-case like WorkQueue. We should investigate using them instead of ::WaitForMultipleObjects to see what performance gains we can get.

1. http://msdn.microsoft.com/en-us/library/aa365198(VS.85).aspx
------- Comment #1 From 2010-07-28 14:52:02 PST -------
<rdar://problem/8247273>
------- Comment #2 From 2010-07-30 09:45:47 PST -------
Apparently events can't be associated with I/O completion ports. We currently use events to find out when read/write operations are completed on the pipe that connects the two processes. If we switched to using two unidirectional pipes, we could wait on the pipes themselves, and thus use them with an I/O completion port.
------- Comment #3 From 2010-07-30 09:58:53 PST -------
(In reply to comment #2)
> If we switched to using two unidirectional pipes, we could wait on the pipes themselves, and thus use them with an I/O completion port.

Or maybe we could still use a single duplex pipe but open two handles to it, one that's used for reading and one that's used for writing.
------- Comment #4 From 2010-07-30 10:01:26 PST -------
(In reply to comment #3)
> Or maybe we could still use a single duplex pipe but open two handles to it, one that's used for reading and one that's used for writing.

By "a single duplex pipe" I mean "one pipe with PIPE_ACCESS_DUPLEX".
------- Comment #5 From 2010-07-30 10:11:43 PST -------
You just need a separate OVERLAPPED struct for reads and writes.  One pipe handle is all you need.

You can see how this is implemented here:
http://src.chromium.org/viewvc/chrome/trunk/src/ipc/ipc_channel_win.cc?view=markup
------- Comment #6 From 2010-07-30 10:22:49 PST -------
(In reply to comment #5)
> You just need a separate OVERLAPPED struct for reads and writes.  One pipe handle is all you need.
> 
> You can see how this is implemented here:
> http://src.chromium.org/viewvc/chrome/trunk/src/ipc/ipc_channel_win.cc?view=markup

We currently have one pipe handle and two OVERLAPPED structs. We use events in the OVERLAPPED structs to find out when the I/O is complete. In comment 2 I was realizing that we can't associate those events with an I/O completion port. But I guess you're saying we can stop using events and just compare the addresses of the OVERLAPPED structs to figure out what happened.
------- Comment #7 From 2010-08-18 09:55:02 PST -------
I have a working patch for this. As a bonus, it fixes bug 42826, too!
------- Comment #8 From 2010-08-18 10:16:08 PST -------
I'm using a completion port to fix bug 42826, so duping to that.

*** This bug has been marked as a duplicate of bug 42826 ***
------- Comment #9 From 2010-08-26 14:44:31 PST -------
(In reply to comment #8)
> I'm using a completion port to fix bug 42826, so duping to that.
> 
> *** This bug has been marked as a duplicate of bug 42826 ***

The plan has changed. Reopening this bug.
------- Comment #10 From 2010-08-31 09:40:12 PST -------
We could use BindIoCompletionCallback <http://msdn.microsoft.com/en-us/library/aa363484(v=VS.85).aspx> to use the thread pool's global completion port. That would probably be better than using our own per-WorkQueue port.
------- Comment #11 From 2010-09-07 11:34:42 PST -------
Unfortunately, BindIOCompletionCallback won't allow us to use PostQueuedCompletionStatus to perform non-I/O work. If we were to use our own I/O completion port we could do that.
------- Comment #12 From 2010-09-08 12:11:04 PST -------
(In reply to comment #11)
> Unfortunately, BindIOCompletionCallback won't allow us to use PostQueuedCompletionStatus to perform non-I/O work. If we were to use our own I/O completion port we could do that.

But that's OK, as we can just use ::QueueUserWorkItem instead.
------- Comment #13 From 2010-09-30 07:33:02 PST -------
Unfortunately, BindIoCompletionCallback doesn't give us a way to map back to the triggering HANDLE, unless we know upfront all the OVERLAPPED structs that will be used with a given HANDLE.
------- Comment #14 From 2010-10-08 09:45:46 PST -------
(In reply to bug 42826 comment #7)
> I've been working on switching over to using an I/O completion port in WorkQueue. This would make it unnecessary to unregister a HANDLE; you can just close it and stop worrying.
> 
> However, I'm seeing spurious reads in the web process. I.e., the web process will request 1 byte of the next message from the pipe, and then will be told that 207 bytes were read. This doesn't make any sense! Windows shouldn't be reading more than we asked it to; where would it store the data?

Maybe the problems were due to the reads completing synchronously? My recollection is that I was handling synchronous reads synchronously, but <http://blogs.msdn.com/b/oldnewthing/archive/2010/10/08/10073039.aspx> says that you can still wait for the HANDLE to be signaled. Maybe that's what we need to do.
------- Comment #15 From 2010-12-17 10:45:17 PST -------
(In reply to comment #13)
> Unfortunately, BindIoCompletionCallback doesn't give us a way to map back to the triggering HANDLE, unless we know upfront all the OVERLAPPED structs that will be used with a given HANDLE.

<http://blogs.msdn.com/b/oldnewthing/archive/2010/12/17/10106259.aspx> suggests making a new struct that derives from OVERLAPPED and using that the store whatever extra information you need. We could do that if WorkQueue were in charge of allocating OVERLAPPEDs for clients.