1188690 - Create an Event Loop abstraction in MFBT

Status: NEW

(Core :: MFBT, defect)

Description

[Bobby Holley (:bholley)]

As part of the effort to exposed MozPromise to the rest of Gecko, we need to hoist AbstractThread out of dom/media.

While we're at it, I think it would be super useful to have a thread and event loop abstraction that we could share between spidermonkey and XPCOM. I think hoisting AbstractThread to mfbt would do exactly that, so I'm going to see if I can make it happen.

Comment 1

[Mike Hommey [:glandium]]

mozglue/misc would be a better place for this, without having to care about the implications of adding dependencies to mfbt. It's available to spidermonkey the same way mfbt is.

Comment 2

[Bobby Holley (:bholley)]

(In reply to Mike Hommey [:glandium] from comment #1)
> mozglue/misc would be a better place for this, without having to care about
> the implications of adding dependencies to mfbt. It's available to
> spidermonkey the same way mfbt is.

Can you explain this more? What are these implications, and how do they differ between mozglue and mfbt?

Comment 3

[Mike Hommey [:glandium]]

As mentioned at Whistler, there are things that depend on mfbt implicitly but don't and can't depend on other things like nspr, xpcom, etc. Adding headers to mfbt is fine, but as soon as we touch .cpp files, that becomes complicated.

What's explicitly linked to js and libxul, though, is mozglue, which happens to contain mfbt.

At the moment, mozglue/misc contains mozilla::TimeStamp and the stack walker.

Comment 4

[Bobby Holley (:bholley)]

I think I can do it all in the header. The implementations of the interface can live in XPCOM etc.

Comment 5

[Mike Hommey [:glandium]]

If spindermonkey is going to need the implementations in XPCOM, that's going to work in Firefox, but not in standalone spidermonkey.

Comment 6

[(no longer active)]

(In reply to Mike Hommey [:glandium] from comment #5)
> If spindermonkey is going to need the implementations in XPCOM, that's going
> to work in Firefox, but not in standalone spidermonkey.

js.cpp can provide its own implementation.

Comment 7

[Mike Hommey [:glandium]]

(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #6)
> (In reply to Mike Hommey [:glandium] from comment #5)
> > If spindermonkey is going to need the implementations in XPCOM, that's going
> > to work in Firefox, but not in standalone spidermonkey.
> 
> js.cpp can provide its own implementation.

But why should we have several implementations? (and the hypothetical one for js.cpp would also need to be shared with gdb-tests.cpp and jsapi-tests.cpp)

Comment 8

[Bobby Holley (:bholley)]

Because the actual implementations will be different. The XPCOM one will be tied into XPCOM event loops, whereas the standalone JS one will be much simpler.

Comment 9

[(no longer active)]

Yes, the implementations are *so* different that trying to share them seems pointless.

If in the future we end up with a shared implementation we can put that in mozglue.

Comment 10

[Bobby Holley (:bholley)]

We discussed this on IRC a bit last week.

The conclusion was that we need something separate from AbstractThread (but from which AbstractThread could inherit) in MFBT. The interface would be very minimal - dispatching runnables, and asking "are we currently running in this thing?".

We could call this mozilla::LogicalThread or mozilla::EventLoop.

The tricky bit is defining how this interacts with runnables and reference counting. We presumably need a mozilla::Runnable interface as well, but the details of that are tricky.

Ideally {LogicalThread,EventLoop}::Dispatch would take an argument that is coercible from already_AddRefed<nsIRunnable>. But this means that we need nsIRunnable to inherit from mozilla::Runnable, which gets tricky given the XPIDLiness. Ehsan's suggestion was to do what nsISupports.idl does, and #ifdef-out the generated C++ class and replace it with our own.

This also means that we need to do one of the following:
(1) Call mozilla::Runnable's execution method something other than Run(), and have it forward to Run(). This adds an extra virtual call.
(2) Share the same implementation, which would mean adding something nsresult-y to the MFBT interface, and relying on vtable ordering.
(3) Give the XPIDL Run() method a different binary name, and make _that_ forward to the void-valued Run() in the runnables we care about.

Then we also need to sort out reference counting.

Realistically, I don't have time to work on this right now.