Bug 130557 – [WK2] Make Connection::sendMessage a little less heavy

• Status: RESOLVED
• Resolution: WONTFIX
• Component: New Bugs

[Benjamin Poulain]

[WK2] Make Connection::sendMessage a little less heavy

[Benjamin Poulain]

Created attachment 227376 [details]
Patch

[Anders Carlsson]

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.

[Benjamin Poulain]

(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.

[Anders Carlsson]

(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.

[Benjamin Poulain]

(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?

[Darin Adler]

Anders, could do either review+ or review- on this? It’s been sitting around for a few months now.

[Brent Fulgham]

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? ;-)

[Anders Carlsson]

I think that using Lock instead of Mutex makes this patch unnecessary actually.