Bug 130557

Summary: [WK2] Make Connection::sendMessage a little less heavy
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED WONTFIX    
Severity: Normal CC: andersca, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch bfulgham: review-

Description Benjamin Poulain 2014-03-20 21:06:22 PDT 
[WK2] Make Connection::sendMessage a little less heavy
Comment 1 Benjamin Poulain 2014-03-20 21:11:45 PDT 
Created attachment 227376 [details]
Patch
Comment 2 Anders Carlsson 2014-03-21 07:13:14 PDT 
Comment on attachment 227376 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=227376&action=review

> Source/WebKit2/ChangeLog:8
> +        Dispatching messages on the main thread takes a long time. This reduces the overhead a bit.

This is not true though, we're not dispatching messages on the main thread.
Comment 3 Benjamin Poulain 2014-03-21 09:41:51 PDT 
(In reply to comment #2)
> > Source/WebKit2/ChangeLog:8
> > +        Dispatching messages on the main thread takes a long time. This reduces the overhead a bit.
> 
> This is not true though, we're not dispatching messages on the main thread.

You are thinking dispatching messages to the other process... What takes lots of time here on the main thread is dispatching the messages to the dispatch queue for the secondary thread.
Comment 4 Anders Carlsson 2014-03-21 10:12:24 PDT 
(In reply to comment #3)
> (In reply to comment #2)
> > > Source/WebKit2/ChangeLog:8
> > > +        Dispatching messages on the main thread takes a long time. This reduces the overhead a bit.
> > 
> > This is not true though, we're not dispatching messages on the main thread.
> 
> You are thinking dispatching messages to the other process... What takes lots of time here on the main thread is dispatching the messages to the dispatch queue for the secondary thread.

Then the ChangeLog should make that clear.

I don't think this patch is right either, but I'd have to look closer at the code.
Comment 5 Benjamin Poulain 2014-03-21 17:20:01 PDT 
(In reply to comment #4)
> > > This is not true though, we're not dispatching messages on the main thread.
> > 
> > You are thinking dispatching messages to the other process... What takes lots of time here on the main thread is dispatching the messages to the dispatch queue for the secondary thread.
> 
> Then the ChangeLog should make that clear.

The ChangeLog never said anything about dispatching messages between processes :)

> I don't think this patch is right either, but I'd have to look closer at the code.

Can you please check?
Comment 6 Darin Adler 2014-07-12 16:50:52 PDT 
Anders, could do either review+ or review- on this? It’s been sitting around for a few months now.
Comment 7 Brent Fulgham 2016-03-14 11:14:44 PDT 
Comment on attachment 227376 [details]
Patch

Marking patch r- due to its age. Ben, can you update for current sources and we can sit and stare at Anders until he reviews the patch? ;-)
Comment 8 Anders Carlsson 2016-03-14 11:48:13 PDT 
I think that using Lock instead of Mutex makes this patch unnecessary actually.