Closed Bug 1706374 Opened 6 months ago Closed 4 months ago

Use a message router for primary child process channels

Categories

(Core :: IPC, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
91 Branch
Tracking Status
firefox91 --- fixed

People

(Reporter: nika, Assigned: nika)

References

(Blocks 1 open bug)

Details

Crash Data

Attachments

(16 files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

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.

Depends on: 1706375
See Also: → 1706376

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

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

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.

Mechanical change applying clang-tidy fixes

Mechanical change applying clang-tidy fixes

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.

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

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.

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.

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.

Blocks: 1713148
Attachment #9217119 - Attachment description: Bug 1706374 - Part 10: Add NodeController component bridging IPC and Ports, → Bug 1706374 - Part 11: Add NodeController component bridging IPC and Ports,
Attachment #9217120 - Attachment description: Bug 1706374 - Part 11a: Initialize NodeController when creating IO thread, → Bug 1706374 - Part 12a: Initialize NodeController when creating IO thread,
Attachment #9217121 - Attachment description: Bug 1706374 - Part 11b: Use NodeController for primary process channels, → Bug 1706374 - Part 12b: Use NodeController for primary process channels,

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

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.

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

Pushed by nlayzell@mozilla.com:
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
Regressions: 1717547
Pushed by nlayzell@mozilla.com:
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
Regressions: 1718348
Crash Signature: [@ libxul.so (deleted)@0x67ec3e0] [@ libxul.so (deleted)@0xb34edf] [@ libxul.so (deleted)@0xf9a35c] [@ libxul.so (deleted)@0xf9e40c] [@ libxul.so (deleted)@0xfb853f] [@ mozilla::dom::TCPSocket::cycleCollection::TraverseNative] [@ mozilla::dom::indexedDB::…
You need to log in before you can comment on or make changes to this bug.