Mojo: Use new message APIs to reduce copying

Mojo: Use new message APIs to reduce copying

- Adds new public C++ APIs for scoped message objects
- Updates the bindings to use new Write/ReadMessage APIs
- Updates ChannelMojo to use new WriteMessage API

This results in one fewer copy on either end of every Mojo IPC,
whether it's over ChannelMojo or an arbitrary message pipe.

BUG=590495
R=yzshen@chromium.org

Committed: https://crrev.com/596231c83a488a54e994a43417e617c864730b3a
Cr-Commit-Position: refs/heads/master@{#390709}

[Ken Rockot(use gerrit already), 2016-04-28 22:44:15]

Description was changed from

==========
Mojo: Use new message APIs to reduce copying

- Updates the bindings to use new Write/ReadMessage APIs
- Updates ChannelMojo to use new WriteMessage API

This results in one fewer copy on either end of every Mojo IPC,
whether it's over ChannelMojo or an arbitrary message pipe.

BUG=590495
R=yzshen@chromium.org
==========

to

==========
Mojo: Use new message APIs to reduce copying

- Adds new public C++ APIs for scoped message objects
- Updates the bindings to use new Write/ReadMessage APIs
- Updates ChannelMojo to use new WriteMessage API

This results in one fewer copy on either end of every Mojo IPC,
whether it's over ChannelMojo or an arbitrary message pipe.

BUG=590495
R=yzshen@chromium.org
==========

[Ken Rockot(use gerrit already), 2016-04-28 22:46:54]

Yuzhu, please take a look.

I have opted to eliminate PickleBuffer, which means native struct pickles are
now copied into the buffer during serialization, rather than being written
in-place by ParamTraits. I think this is a worthwhile trade-off since usage
should be minimal and it reduces complexity quite a bit. WDYT?

For follow-up work I think I can further eliminate copies from ChannelMojo's
MessagePipeReader.

[yzshen1, 2016-04-29 16:50:20]

LGTM with a few nits.

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/message.cc (right):

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/message.cc:58: // behavior to collect handle
counts. It's unoptimized for now because it's
According to my understanding, it needs to collect handles instead of handle
counts.
Agreed that it would be nice but we could wait.

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/message_buffer.cc (right):

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/message_buffer.cc:37: MojoResult rv =
GetMessageBuffer(message_.get(), &buffer_);
I noticed that the previous method explicitly set buffer_ to null if
data_num_bytes_ is 0.
Please make them consistent. (Either way seems safe.)

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/message_buffer.cc:55: char* start =
static_cast<char*>(buffer_) + bytes_claimed_;
It relies on the fact that |buffer_| itself is 8-byte aligned.
Is that guaranteed by the edk implementation? (I guess yes.) If yes, please
comment it explicitly in the API documentation.

[Ken Rockot(use gerrit already), 2016-04-29 17:37:13]

Thanks!

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/message.cc (right):

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/message.cc:58: // behavior to collect handle
counts. It's unoptimized for now because it's
On 2016/04/29 at 16:50:20, yzshen1 wrote:
> According to my understanding, it needs to collect handles instead of handle
counts.
> Agreed that it would be nice but we could wait.

Fixed comment

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
File mojo/public/cpp/bindings/lib/message_buffer.cc (right):

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/message_buffer.cc:37: MojoResult rv =
GetMessageBuffer(message_.get(), &buffer_);
On 2016/04/29 at 16:50:20, yzshen1 wrote:
> I noticed that the previous method explicitly set buffer_ to null if
data_num_bytes_ is 0.
> Please make them consistent. (Either way seems safe.)

done

https://codereview.chromium.org/1932083002/diff/1/mojo/public/cpp/bindings/li...
mojo/public/cpp/bindings/lib/message_buffer.cc:55: char* start =
static_cast<char*>(buffer_) + bytes_claimed_;
On 2016/04/29 at 16:50:20, yzshen1 wrote:
> It relies on the fact that |buffer_| itself is 8-byte aligned.
> Is that guaranteed by the edk implementation? (I guess yes.) If yes, please
comment it explicitly in the API documentation.

done

[Ken Rockot(use gerrit already), 2016-04-29 17:37:16]

The CQ bit was checked by rockot@chromium.org

[Ken Rockot(use gerrit already), 2016-04-29 17:37:16]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org
Link to the patchset: https://codereview.chromium.org/1932083002/#ps20001
(title: "nits")

[commit-bot: I haz the power, 2016-04-29 17:37:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1932083002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1932083002/20001

[commit-bot: I haz the power, 2016-04-29 18:46:56]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-30 17:27:39]

Patchset 2 (id:??) landed as
https://crrev.com/596231c83a488a54e994a43417e617c864730b3a
Cr-Commit-Position: refs/heads/master@{#390709}

[commit-bot: I haz the power, 2016-04-30 18:01:38]

Description was changed from

==========
Mojo: Use new message APIs to reduce copying

- Adds new public C++ APIs for scoped message objects
- Updates the bindings to use new Write/ReadMessage APIs
- Updates ChannelMojo to use new WriteMessage API

This results in one fewer copy on either end of every Mojo IPC,
whether it's over ChannelMojo or an arbitrary message pipe.

BUG=590495
R=yzshen@chromium.org
==========

to

==========
Mojo: Use new message APIs to reduce copying

- Adds new public C++ APIs for scoped message objects
- Updates the bindings to use new Write/ReadMessage APIs
- Updates ChannelMojo to use new WriteMessage API

This results in one fewer copy on either end of every Mojo IPC,
whether it's over ChannelMojo or an arbitrary message pipe.

BUG=590495
R=yzshen@chromium.org

Committed: https://crrev.com/596231c83a488a54e994a43417e617c864730b3a
Cr-Commit-Position: refs/heads/master@{#390709}
==========