Bug 43148

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

Description Adam Roben (:aroben) 2010-07-28 14:48:57 PDT
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 Adam Roben (:aroben) 2010-07-28 14:52:02 PDT
<rdar://problem/8247273>
Comment 2 Adam Roben (:aroben) 2010-07-30 09:45:47 PDT
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 Adam Roben (:aroben) 2010-07-30 09:58:53 PDT
(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 Adam Roben (:aroben) 2010-07-30 10:01:26 PDT
(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 Darin Fisher (:fishd, Google) 2010-07-30 10:11:43 PDT
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 Adam Roben (:aroben) 2010-07-30 10:22:49 PDT
(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 Adam Roben (:aroben) 2010-08-18 09:55:02 PDT
I have a working patch for this. As a bonus, it fixes bug 42826, too!
Comment 8 Adam Roben (:aroben) 2010-08-18 10:16:08 PDT
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 Adam Roben (:aroben) 2010-08-26 14:44:31 PDT
(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 Adam Roben (:aroben) 2010-08-31 09:40:12 PDT
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 Adam Roben (:aroben) 2010-09-07 11:34:42 PDT
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 Adam Roben (:aroben) 2010-09-08 12:11:04 PDT
(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 Adam Roben (:aroben) 2010-09-30 07:33:02 PDT
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 Adam Roben (:aroben) 2010-10-08 09:45:46 PDT
(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 Adam Roben (:aroben) 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.