Dart: Uses a pre-allocated buffer for message pipe query and read.

Dart: Uses a pre-allocated buffer for message pipe query and read.

BUG=
R=johnmccutchan@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/1e3d7866619c1f9df6a077d6748e580f6b93ff1b

[zra, 2015-10-16 20:24:05]

zra@google.com changed reviewers:
+ johnmccutchan@google.com

[Cutch, 2015-10-16 20:50:56]

LGTM with one comment

https://codereview.chromium.org/1410053002/diff/20001/mojo/public/dart/mojo/l...
File mojo/public/dart/mojo/lib/src/message_pipe.dart (right):

https://codereview.chromium.org/1410053002/diff/20001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/message_pipe.dart:31: List _result;
This could be static and allocated only once right?

[zra, 2015-10-16 21:07:11]

https://codereview.chromium.org/1410053002/diff/20001/mojo/public/dart/mojo/l...
File mojo/public/dart/mojo/lib/src/message_pipe.dart (right):

https://codereview.chromium.org/1410053002/diff/20001/mojo/public/dart/mojo/l...
mojo/public/dart/mojo/lib/src/message_pipe.dart:31: List _result;
On 2015/10/16 20:50:55, Cutch wrote:
> This could be static and allocated only once right?

Done.

[zra, 2015-10-16 21:18:03]

Committed patchset #3 (id:40001) manually as
1e3d7866619c1f9df6a077d6748e580f6b93ff1b (presubmit successful).

[jamesr, 2015-10-19 21:43:15]

jamesr@chromium.org changed reviewers:
+ jamesr@chromium.org

[jamesr, 2015-10-19 21:43:15]

Does this mean that we keep a buffer around the size of the largest mojo message
ever received for the life of the isolate?

[zra, 2015-10-19 22:00:00]

On 2015/10/19 21:43:15, jamesr wrote:
> Does this mean that we keep a buffer around the size of the largest mojo
message
> ever received for the life of the isolate?

Yes.

[jamesr, 2015-10-19 22:11:20]

Is there a plan to improve that or some sort of discussion somewhere I can see? 
There's no bug linked to the patch.  Leaving several megabytes (potentially)
sitting around per isolate seems pretty unfortunate without a very compelling
benefit.

[zra, 2015-10-19 22:53:53]

On 2015/10/19 22:11:20, jamesr wrote:
> Is there a plan to improve that or some sort of discussion somewhere I can
see? 
> There's no bug linked to the patch.  Leaving several megabytes (potentially)
> sitting around per isolate seems pretty unfortunate without a very compelling
> benefit.

The discussion was in offline between me and John. We have a general effort to
reduce the time of IPC calls in Dart. Here are the stats before/after this
change:

before:

avg_duration/mojo_rtt_benchmark/ping 0.153223
percentile_duration/mojo_rtt_benchmark/ping/0.10 0.095
percentile_duration/mojo_rtt_benchmark/ping/0.50 0.137
percentile_duration/mojo_rtt_benchmark/ping/0.90 0.233

after:

avg_duration/mojo_rtt_benchmark/ping 0.142605
percentile_duration/mojo_rtt_benchmark/ping/0.10 0.092
percentile_duration/mojo_rtt_benchmark/ping/0.50 0.123
percentile_duration/mojo_rtt_benchmark/ping/0.90 0.225

So this change shaves ~10% off the median IPC Round Trip time.

We have not yet discussed ways to improve on this patch.

[jamesr, 2015-10-20 21:40:01]

It would be really useful to capture data like this in a bug and reference that
from the patch.  I can file one for this for tracking