Optimize the message queue for many active ports with few messages.

Optimize the message queue for many active ports with few messages.

The current message queue implementation is very inefficient if you are using
many active ports with few messages. This happens when you use 'call' a lot
on ports which is currently done in dart:io.

This patch does not remove messages from the queue when closing a port. Instead
it drops messages for closed ports when it processes them. This dramatically
speeds up dart:io benchmarks that enqueue tons of writes on a file output
stream.

R=iposva@google.com
BUG=http://dartbug.com/6911

Committed: https://code.google.com/p/dart/source/detail?r=16388

[Mads Ager (google), 2012-12-12 07:40:05]

ping

[Ivan Posva, 2012-12-13 02:26:54]

I am not sure that this code properly drains the message queue when the last
remaining port for a message handler is closed or whether it leaks pending
messages for closed ports.

Also we should not consult the portmap repeatedly and instead rely on the
port->handler map in the isolate for non-native ports.

-Ivan

[Mads Ager (google), 2012-12-14 13:53:29]

Thanks for the comments Ivan.

I have changed the code to perform the lookup in the port map in the dart code
instead of the complicated locking on the port map on the C side.

Regarding cleanup: I have been unable to create an example that doesn't cleanup
properly. In the MessageQueue destructor there is an ASSERT that there are no
messages. When shutting down an isolate, we clear all messages from the message
queue. As far as I can tell, when closing a port, we could get into a situation
where we call the MessageQueue destructor without clearing all messages.
However, I cannot figure out how to trigger that. Any advice on that would be
greatly appreciated. Maybe it can't happen, but I don't understand the logic
well enough to understand why.

[Ivan Posva, 2012-12-19 07:09:44]

Looks much better, but what I am not 100% convinced yet is that we will never
close a message handler which has still some pending messages. This is
especially the case when closing a native port, which will do
MessageHandler::ClosePort.

-Ivan

https://codereview.chromium.org/11440035/diff/5001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/11440035/diff/5001/runtime/vm/isolate.cc#newc...
runtime/vm/isolate.cc:92: Object& receiver = Object::Handle();
I find the name choice of receiver a bit confusing. How about receive_port?

[Mads Ager (google), 2012-12-19 15:47:57]

I agree that there should be a case where we can destruct the message handler
without clearing messages. We will always destruct the message handler. Instead
of just deleting the queues in the destructor I have added clearing of the
queues first which will delete all the messages.

https://codereview.chromium.org/11440035/diff/5001/runtime/vm/isolate.cc
File runtime/vm/isolate.cc (right):

https://codereview.chromium.org/11440035/diff/5001/runtime/vm/isolate.cc#newc...
runtime/vm/isolate.cc:92: Object& receiver = Object::Handle();
On 2012/12/19 07:09:44, Ivan Posva wrote:
> I find the name choice of receiver a bit confusing. How about receive_port?

Done.

[Mads Ager (google), 2012-12-19 18:36:39]

Moved the clearing to the MessageQueue destructor as discussed offline. PTAL

[Ivan Posva, 2012-12-20 17:19:43]

LGTM with comment.

-Ivan

https://codereview.chromium.org/11440035/diff/13001/runtime/vm/dart_entry.cc
File runtime/vm/dart_entry.cc (right):

https://codereview.chromium.org/11440035/diff/13001/runtime/vm/dart_entry.cc#...
runtime/vm/dart_entry.cc:321: RawObject*
DartLibraryCalls::LookupReceivePort(Dart_Port port_id) {
Please coordinate with +fschneider as he has relevant changes in this general
area.

[Mads Ager (google), 2012-12-20 19:14:47]

Thanks Ivan. I'll merge with Florian's caching changes. :-)