1621762 - Add IpdlQueue option for remote WebGL

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, enhancement, P1)

Description

[David Parks [:handyman]]

The current remote WebGL framework is anchored with the ProducerConsumerQueue. That object uses its own synchronization to implement a queue of data shared between the content and compositor processes. It also currently suffers from incomplete implementations on Linux and Mac. IpdlQueue, instead, is implemented with IPDL messages, which work on all supported platforms. The queue offers the same interfaces so it works as a drop-in replacement. IPDL actors that will be used as foundation for the queue need to implement provided traits (classes with a <Derived> template parameter) that provide them with the new functionality.

This bug is just for integrating the new class. There are still other issues with remote WebGL that will keep it from functioning fully. The IpdlQueue is also expected to have some performance issues (in both time and space) but should suffice as an initial solution.

Comment 1

[David Parks [:handyman]]

Attachment: WIP: git-working-ipdlq.patch

WIP implementation of a working IpdlQueue against the old working WebGL remoting repo.

Comment 2

[David Parks [:handyman]]

Attachment: Bug 1621762 - Part 1: Basic refactoring of ProducerConsumerQueue & CommandQueue r=jgilbert!

A lot of simple changes to prepare for later patches in the series:

• PcqCommand -> QueueCommand, PcqStatus -> QueueStatus, PcqParamTraits -> QueueParamTraits
• Consolidate BasicSink/Source into CommandSink/Source
• SyncCommandSource::mConsumer -> mResponseSink, SyncCommandSink::mProducer -> mResponseSource, CommandSource::mProducer -> mSource, CommandSink::mConsumer -> mSink
• Rename WebGLPcqParamTraits.h to WebGLQueueParamTraits.h
• ProducerConsumerQueue struct -> class.
• ProducerConsumerQueue::mProducer/mConsumer access -> ProducerConsumerQueue::TakeProducer/TakeConsumer
• Rename QueueStatus enum elements to follow style convention.

Comment 3

[David Parks [:handyman]]

Attachment: Bug 1621762 - Part 2: Refactor WebGL MethodDispatcher code used in remoting r=jgilbert!

• Remove CommandSink from WebGLMethodDispatcher. The parameter is unneeded.
• Make MethodDispatcher code simpler. This removes a layer of complexity (CommandDispatchDriver) involved in finding the correct method for a given ID number.
• Require WebGL methods whose signatures don't allow them to be remoted to directly call on the host object. They are: ReadPixels, GetVRFrame, GetInternalFormatParameter, GetBufferSubData, GetLinkResult and TexImage.
• Fix a bunch of compilation issues with busted PcqParamTraits.
• implement PcqParamTraits for some missing types

Depends on D68256

Comment 4

[David Parks [:handyman]]

Attachment: Bug 1621762: Part 3 - Make WebGL DispatchCommand into a lambda r=jgilbert!

Make CommandSink take lambdas for handling a command or an OOM, instead of using virtual methods. The mixture of template classes and virtual methods can get hairy and this was a bad case. This dramatically simplifies the webgl command classes.

Depends on D68257

Comment 5

[David Parks [:handyman]]

Attachment: Bug 1621762: Part 3 - Make some classes generic to support multiple remote WebGL implementations r=jgilbert!

• Templatize ProducerView/ConsumerView/(Sync)CommandSource/(Sync)CommandSink to allow both Pcq and Ipdl versions, as the client code is identical.
• Rename Producer/Consumer -> PcqProducer/PcqConsumer.

Depends on D68258

Comment 6

[David Parks [:handyman]]

Attachment: Bug 1621762: Part 4 - Move some code from ProducerConsumerQueue.h to QueueParamTraits.h r=jgilbert!

Move QueueParamTraits, which are common between PCQ and IpdlQueue, to a file they both include. This also changes the namespace of a handful of the classes, like the Marshaller.

Depends on D68259

Comment 7

[David Parks [:handyman]]

Attachment: Bug 1621762: Part 5 - Change PWebGL alloc+constructor to Initialize message r=jgilbert!

We need to separate WebGL actor construction and initialization since IpdlQueue initialization needs the actor to already exist.

Depends on D68260

Comment 8

[David Parks [:handyman]]

Attachment: Bug 1621762: Part 6 - Add and define IpdlQueue for WebGL IPC r=jgilbert!

The IpdlQueue (de)serializes types into arrays that are then passed to another process via IPDL messages. This means much less bloat than generating IPDL routines for each of the commands we send with WebGL. This IPDL queue is fairly basic -- it simply sends "async" commands through an async IPDL message and sync commands through a sync message. Future extensions, such as a facility for buffering async messages to be sent in bulk, are planned.
The IpdlQueue uses an existing actor to send messages. That actor should derive from one of the provided base classes and implement any needed IPDL message. For example, if the actor is used as the comsumer for sync messages then it should subclass SyncConsumerActor and should Recv (via IPDL) the ExchangeIpdlQueueData message -- the handler method for which is defined in the SyncProducerActor base class.

Depends on D68262

Comment 9

[David Parks [:handyman]]

Attachment: Bug 1621762: Part 7 - Add IpdlQueue actor traits to WebGLParent/WebGLChild r=jgilbert!

Adds IpdlQueue capability to PWebGL actors. The WebGLChild, used in content processes, implements SyncProducerActor and AsyncConsumerActor because it sends (sync and async) messages and receives responses to them that it reads as async messages. The WebGLParent, used in the compositor process, is a SyncConsumerActor and AsyncProducerActor for dual reasons.

Depends on D68263

Comment 10

[David Parks [:handyman]]

For prosperity, since abandoned revisions are removed from the bug, note that I removed the old part 3, which made DispatchCommand a lambda. See https://phabricator.services.mozilla.com/D68258

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d85420fd2e6
Part 1: Basic refactoring of ProducerConsumerQueue & CommandQueue r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/0496adcb4582
Part 2: Refactor WebGL MethodDispatcher code used in remoting r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/010f653b87d2
Part 3 - Make some classes generic to support multiple remote WebGL implementations r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/a3e1332998c3
Part 4 - Move some code from ProducerConsumerQueue.h to QueueParamTraits.h r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/70d103786c83
Part 5 - Change PWebGL alloc+constructor to Initialize message r=jgilbert,jld
https://hg.mozilla.org/integration/autoland/rev/21ef72486643
Part 6 - Add and define IpdlQueue for WebGL IPC r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/03903e8f368e
Part 7 - Add IpdlQueue actor traits to WebGLParent/WebGLChild r=jgilbert,jld

Comment 12

[Daniel Varga [:dvarga]]

Backed out 7 changesets (Bug 1621762) for causing build bustages at builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=AS60ai1rQ8mRy_cBIg3xhA-0&resultStatus=testfailed%2Cbusted%2Cexception&revision=03903e8f368ef71e0624daa0a960b3b35d5518f2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=300134986&repo=autoland&lineNumber=21844

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=f5b6bbac55bc2d9e8f49e7fdc21dcfccd03e3952

[task 2020-04-30T02:57:22.199Z] 02:57:22     INFO -  In file included from /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h:10:0,
[task 2020-04-30T02:57:22.199Z] 02:57:22     INFO -                   from /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCrossProcessCommandQueue.h:9,
[task 2020-04-30T02:57:22.199Z] 02:57:22     INFO -                   from /builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders/mozilla/dom/PWebGLParent.h:21,
[task 2020-04-30T02:57:22.200Z] 02:57:22     INFO -                   from PCompositorBridge.cpp:26,
[task 2020-04-30T02:57:22.200Z] 02:57:22     INFO -                   from UnifiedProtocols11.cpp:83:
[task 2020-04-30T02:57:22.200Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h:815:13: error: explicit specialization in non-namespace scope 'class mozilla::webgl::PcqConsumer'
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -     template <>
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -               ^
[task 2020-04-30T02:57:22.201Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h:823:15: error: too many template-parameter-lists
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -     QueueStatus TryCopyOrSkipItem(ConsumerView<PcqConsumer>& aView, Arg* aArg) {
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -                 ^~~~~~~~~~~~~~~~~
[task 2020-04-30T02:57:22.201Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h:829:15: error: too many template-parameter-lists
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -     QueueStatus ReadObject(size_t* aRead, size_t aWrite, Arg* arg,
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -                 ^~~~~~~~~~
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h: In member function 'mozilla::webgl::QueueStatus mozilla::webgl::PcqConsumer::TryPeekRemoveHelper(mozilla::webgl::ConsumerView<mozilla::webgl::PcqConsumer>&, Arg*, Args* ...)':
[task 2020-04-30T02:57:22.201Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h:810:26: error: 'TryCopyOrSkipItem' was not declared in this scope
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -       QueueStatus status = TryCopyOrSkipItem<Arg>(aView, aArg);
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -                            ^~~~~~~~~~~~~~~~~
[task 2020-04-30T02:57:22.201Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/ProducerConsumerQueue.h:810:47: error: expected primary-expression before '>' token
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -       QueueStatus status = TryCopyOrSkipItem<Arg>(aView, aArg);
[task 2020-04-30T02:57:22.201Z] 02:57:22     INFO -                                                 ^
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -  In file included from /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCrossProcessCommandQueue.h:9:0,
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -                   from /builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders/mozilla/dom/PWebGLParent.h:21,
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -                   from PCompositorBridge.cpp:26,
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -                   from UnifiedProtocols11.cpp:83:
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h: At global scope:
[task 2020-04-30T02:57:22.202Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h:513:13: error: explicit specialization in non-namespace scope 'struct mozilla::MethodDispatcher<Derived>'
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -     template <>
[task 2020-04-30T02:57:22.202Z] 02:57:22     INFO -               ^
[task 2020-04-30T02:57:22.202Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h:514:10: error: template parameters not deducible in partial specialization:
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -     struct DispatchMethod<CommandSyncType::ASYNC> {
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h:514:10: note:         'Derived'
[task 2020-04-30T02:57:22.203Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h:523:13: error: explicit specialization in non-namespace scope 'struct mozilla::MethodDispatcher<Derived>'
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -     template <>
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -               ^
[task 2020-04-30T02:57:22.203Z] 02:57:22    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/dom/WebGLCommandQueue.h:524:10: error: template parameters not deducible in partial specialization:
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -     struct DispatchMethod<CommandSyncType::SYNC> {
[task 2020-04-30T02:57:22.203Z] 02:57:22     INFO -            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Comment 13

[David Parks [:handyman]]

Trying again.

https://treeherder.mozilla.org/#/jobs?repo=try&selectedTaskRun=LsSvcVWGQWu_0LUjuyBE1A-0&revision=3343276452723ac7e7ab1a8b0ed75949a49518bf

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c1234ac0e809
Part 1: Basic refactoring of ProducerConsumerQueue & CommandQueue r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/dc396e24598e
Part 2: Refactor WebGL MethodDispatcher code used in remoting r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/1e7653cf240b
Part 3 - Make some classes generic to support multiple remote WebGL implementations r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/d81962dece06
Part 4 - Move some code from ProducerConsumerQueue.h to QueueParamTraits.h r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/a789468a1d87
Part 5 - Change PWebGL alloc+constructor to Initialize message r=jgilbert,jld
https://hg.mozilla.org/integration/autoland/rev/1049dadfd029
Part 6 - Add and define IpdlQueue for WebGL IPC r=jgilbert
https://hg.mozilla.org/integration/autoland/rev/bfbe58c43814
Part 7 - Add IpdlQueue actor traits to WebGLParent/WebGLChild r=jgilbert,jld

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/c1234ac0e809
https://hg.mozilla.org/mozilla-central/rev/dc396e24598e
https://hg.mozilla.org/mozilla-central/rev/1e7653cf240b
https://hg.mozilla.org/mozilla-central/rev/d81962dece06
https://hg.mozilla.org/mozilla-central/rev/a789468a1d87
https://hg.mozilla.org/mozilla-central/rev/1049dadfd029
https://hg.mozilla.org/mozilla-central/rev/bfbe58c43814