Open Bug 1188690 Opened 10 years ago Updated 3 months ago

Create an Event Loop abstraction in MFBT

Categories

(Core :: MFBT, defect)

defect

Tracking

()

People

(Reporter: bholley, Unassigned)

References

Details

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.
Blocks: 1188693
Depends on: 1188696
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.
(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?
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.
I think I can do it all in the header. The implementations of the interface can live in XPCOM etc.
If spindermonkey is going to need the implementations in XPCOM, that's going to work in Firefox, but not in standalone spidermonkey.
(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.
(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)
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.
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.
Blocks: 1188976
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.
Assignee: bobbyholley → nobody
Summary: Move AbstractThread to mfbt → Create an Event Loop abstraction in MFBT
No longer blocks: 1188976
No longer blocks: 1184632
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.