1706374 - Use a message router for primary child process channels

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Nika Layzell [:nika] (ni? for response)]

This patch imports the core algorithms from the mojo/core/ports directory in the chromium tree, and build our own NodeController and NodeChannel objects on top of them. Our objects are simpler and don't support as many features as mojo's system API, but integrate cleanly with our existing infrastructure.

The "ports" directory was imported to keep the amount of code being forked low, to avoid potential issues caused by us not keeping up with upstream. The code in this directory is platform independent, well contained, and has relatively few dependencies on base, so can be more easily forked and integrated into libxul.

Many of the useful features supported by the ports library haven't been implemented in NodeController yet, including sending ports as attachments to other ports, and out-of-order processing of user messages to allow routing handles etc. through the broker.

Currently this is more of an experimental prototype, but the difficulty of the integration and the number of features it provides us makes it seem like a promising approach to fixing bug 1706368.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 1: Import the mojo/core/ports directory from Chromium,

This initial import does not build, and will be made to build in following
parts.

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 2: Format mojo/core/ports with clang-format,

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 3: Get mojo/core/ports building in libxul,

This involves replacing a number of types normally provided by chromium's
base with their XPCOM counterparts etc.

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 4: Run ports gtests in tree,

The ports library import came with 2 gtests. This patch gets them running under
our gtest runner to ensure we don't break the ports functionality.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 5a: Apply suggested clang-tidy fixes to the ports code,

Mechanical change applying clang-tidy fixes

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 5b: Apply suggested clang-tidy fixes to ports,

Mechanical change applying clang-tidy fixes

Comment 7

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 6: Add Mozilla extensions to the Name type,

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 7: Add owning helpers for working with transport descriptors,

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 8: Add support for separate footers to IPC::Message,

These will be used to serialize extra event metadata into IPC messages when
they're sent over the ports infrastructure. In the future better integration
may be used to reduce the overhead of this if necessary.

Comment 10

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 9: Allow reading OtherPid() from an opened IPC::Channel,

This will be used to handle pre-opened channels more reliably in ports code.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 11: Add NodeController component bridging IPC and Ports,

The NodeController and NodeChannel types act as the backbone connecting the
existing IPC logic and driving the ports routing code. Individual NodeChannel
objects wrap and respond to messages from IPC::Channel, and the NodeController
orchestrates all messaging for a process.

The design of these types are inspired by the types with the same names from
Mojo but have been simplified and streamlined to only support features used by
Gecko.

Support for attaching ports or handles to messages hasn't been added yet, but
can be added in follow-up patches.

Comment 12

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 12a: Initialize NodeController when creating IO thread,

This also consumes the existing channel created when launching a process to
create the the conneciton required by NodeController for communicating between
processes. In part 11b, consumers of the broken APIs will be adjusted to use
the new interface.

The new routing approach is not used for the fork server process, as an IO
thread and the NodeController object cannot be initialized before the fork has
been performed, and the IPC requirements of that process are fairly minimal.

Comment 13

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 12b: Use NodeController for primary process channels,

This extends on the changes in part 11a and consumes the new PortRef-based API
in all existing process types other than the fork server. The IPDL C++ unit
tests were already broken before this change, and were not updated.

Comment 14

[Nika Layzell [:nika] (ni? for response)]

fairly-green try: https://treeherder.mozilla.org/jobs?repo=try&revision=5f0f7138455ce92c479d462637828c36eeedd286

Comment 15

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 10: Remove unnecessary IToplevelProtocol::OnChannelConnected,

Comment 16

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 13: Remove the event footer from an IPC::Message when deserializing,

This unfortunately requires a new method to be added to BufferList to
support truncating the buffer to a particular iterator.

Comment 17

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1706374 - Part 14: Track the absolute offset on iterators,

This should improve the performance of large calls to AdvanceAcrossSegments
when using a very large BufferList, as we no longer need to iterate over each
element to find the destination when the call is closer to the end.

This will be used most frequently with the new footer code to seek to the end
of an IPC message to read out the footer.

Comment 18

[Nika Layzell [:nika] (ni? for response)]

Ended up doing some perf runs (talos & vismet browsertime at :jesup's suggestion) with these patches & bug ??? to make sure we weren't causing any serious regressions. They seem somewhat all-over-the-place, but I'm also not the best at reading this perf results :-)

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=6a7940c15631db492010be4b873c74d374daf2d6&newProject=try&newRevision=1977b915b27cef4c0d8f38b52289793f9654c555&framework=1

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b412d6e9e145
Part 1: Import the mojo/core/ports directory from Chromium, r=handyman
https://hg.mozilla.org/integration/autoland/rev/23cd961575a6
Part 2: Format mojo/core/ports with clang-format, r=handyman
https://hg.mozilla.org/integration/autoland/rev/c72c5be9ddb1
Part 3: Get mojo/core/ports building in libxul, r=handyman
https://hg.mozilla.org/integration/autoland/rev/e21b6defb805
Part 4: Run ports gtests in tree, r=handyman
https://hg.mozilla.org/integration/autoland/rev/a6a5d05b5636
Part 5a: Apply suggested clang-tidy fixes to the ports code, r=handyman
https://hg.mozilla.org/integration/autoland/rev/02249671c2f9
Part 5b: Apply suggested clang-tidy fixes to ports, r=handyman
https://hg.mozilla.org/integration/autoland/rev/c6303af17ff4
Part 6: Add Mozilla extensions to the Name type, r=handyman
https://hg.mozilla.org/integration/autoland/rev/ffde07f73249
Part 7: Add owning helpers for working with transport descriptors, r=handyman
https://hg.mozilla.org/integration/autoland/rev/ab2b086abbd4
Part 8: Add support for separate footers to IPC::Message, r=handyman
https://hg.mozilla.org/integration/autoland/rev/110b2384e7d1
Part 9: Allow reading OtherPid() from an opened IPC::Channel, r=handyman
https://hg.mozilla.org/integration/autoland/rev/04422175004b
Part 10: Remove unnecessary IToplevelProtocol::OnChannelConnected, r=handyman,jgilbert
https://hg.mozilla.org/integration/autoland/rev/b09ae4c3a94b
Part 11: Add NodeController component bridging IPC and Ports, r=handyman
https://hg.mozilla.org/integration/autoland/rev/48088463c656
Part 12a: Initialize NodeController when creating IO thread, r=handyman
https://hg.mozilla.org/integration/autoland/rev/72dc6537cf0b
Part 12b: Use NodeController for primary process channels, r=handyman
https://hg.mozilla.org/integration/autoland/rev/c02fa459e77d
Part 13: Remove the event footer from an IPC::Message when deserializing, r=handyman,glandium
https://hg.mozilla.org/integration/autoland/rev/b4c0619e79bf
Part 14: Track the absolute offset on iterators, r=glandium

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f4eeba79d24
Part 1: Import the mojo/core/ports directory from Chromium, r=handyman
https://hg.mozilla.org/integration/autoland/rev/eab1b9e3812c
Part 2: Format mojo/core/ports with clang-format, r=handyman
https://hg.mozilla.org/integration/autoland/rev/7eeaa99e54cb
Part 3: Get mojo/core/ports building in libxul, r=handyman
https://hg.mozilla.org/integration/autoland/rev/e68ae9bac0f4
Part 4: Run ports gtests in tree, r=handyman
https://hg.mozilla.org/integration/autoland/rev/0f54df9a9e71
Part 5a: Apply suggested clang-tidy fixes to the ports code, r=handyman
https://hg.mozilla.org/integration/autoland/rev/b6cffd49343e
Part 5b: Apply suggested clang-tidy fixes to ports, r=handyman
https://hg.mozilla.org/integration/autoland/rev/1a82a6ec42ac
Part 6: Add Mozilla extensions to the Name type, r=handyman
https://hg.mozilla.org/integration/autoland/rev/b0810c6655ed
Part 7: Add owning helpers for working with transport descriptors, r=handyman
https://hg.mozilla.org/integration/autoland/rev/3b2722d8636b
Part 8: Add support for separate footers to IPC::Message, r=handyman
https://hg.mozilla.org/integration/autoland/rev/092bc4ac84dc
Part 9: Allow reading OtherPid() from an opened IPC::Channel, r=handyman
https://hg.mozilla.org/integration/autoland/rev/8bdac5eedf78
Part 10: Remove unnecessary IToplevelProtocol::OnChannelConnected, r=handyman,jgilbert
https://hg.mozilla.org/integration/autoland/rev/7b7819250f79
Part 11: Add NodeController component bridging IPC and Ports, r=handyman
https://hg.mozilla.org/integration/autoland/rev/790fd8710dd7
Part 12a: Initialize NodeController when creating IO thread, r=handyman
https://hg.mozilla.org/integration/autoland/rev/878722e87e62
Part 12b: Use NodeController for primary process channels, r=handyman
https://hg.mozilla.org/integration/autoland/rev/451508713a62
Part 13: Remove the event footer from an IPC::Message when deserializing, r=handyman,glandium
https://hg.mozilla.org/integration/autoland/rev/630d78315203
Part 14: Track the absolute offset on iterators, r=glandium

Comment 21

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/5f4eeba79d24
https://hg.mozilla.org/mozilla-central/rev/eab1b9e3812c
https://hg.mozilla.org/mozilla-central/rev/7eeaa99e54cb
https://hg.mozilla.org/mozilla-central/rev/e68ae9bac0f4
https://hg.mozilla.org/mozilla-central/rev/0f54df9a9e71
https://hg.mozilla.org/mozilla-central/rev/b6cffd49343e
https://hg.mozilla.org/mozilla-central/rev/1a82a6ec42ac
https://hg.mozilla.org/mozilla-central/rev/b0810c6655ed
https://hg.mozilla.org/mozilla-central/rev/3b2722d8636b
https://hg.mozilla.org/mozilla-central/rev/092bc4ac84dc
https://hg.mozilla.org/mozilla-central/rev/8bdac5eedf78
https://hg.mozilla.org/mozilla-central/rev/7b7819250f79
https://hg.mozilla.org/mozilla-central/rev/790fd8710dd7
https://hg.mozilla.org/mozilla-central/rev/878722e87e62
https://hg.mozilla.org/mozilla-central/rev/451508713a62
https://hg.mozilla.org/mozilla-central/rev/630d78315203