Closed
Bug 1458448
Opened 7 years ago
Closed 7 years ago
Create and destroy cooperative CycleCollectedJSContexts where required on the MSG thread
Categories
(Core :: Web Audio, enhancement, P2)
Core
Web Audio
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox61 | --- | affected |
People
(Reporter: karlt, Assigned: karlt)
References
Details
In order to run AudioWorkletProcessor.process() calls synchronously on the MSG
thread, I expect it will be best to use the worklet JS runtime from the MSG thread via CycleCollectedJSContext::InitializeNonPrimary)/JS_NewCooperativeContext().
The MSG thread can change and so new contexts will need to be created and
destroyed when appropriate.
There must not be any observable changes to the global between process() calls
for a single render quantum, and so it would be difficult to make significant
multi-thread gains by processing MessagePort events on another thread, for
example. I suspect it would be simplest and perhaps even best to have the MSG
thread manage the MessagePort event queue, so that it has control. Perhaps it
would be simplest and best to run all worklet thread event queues from the
MSG.
GC, however, could safely occur between process() calls, and so there is the possibility for multi-thread benefits here.
Updated•7 years ago
|
Rank: 15
Priority: -- → P2
Assignee | ||
Comment 1•7 years ago
|
||
Jan, with the changes in
https://hg.mozilla.org/mozilla-central/rev/7db16aa2e0ee
et al, is there a way to use a single JS global object from different threads?
The threads do not need to access the global concurrently, and thread switches
are typically rare.
I wonder whether Quantom DOM's new direction might be useful here?
Would you able to point me at any information on its change of direction,
please?
Flags: needinfo?(jdemooij)
See Also: → 1434211
Comment 2•7 years ago
|
||
Hm that's not really possible because a JSContext is tied to a single thread (and JSRuntime has a pointer to the main context nowadays and JIT code bakes in the JSContext pointer). We could consider adding APIs that let you move a context to a different thread, but we'd rather not do that (if it's even possible).
CC'ing baku and bz, they might have thoughts on this.
> I wonder whether Quantom DOM's new direction might be useful here?
> Would you able to point me at any information on its change of direction,
> please?
As I understand it, cooperative scheduling was not as big of a win as expected and that's why they didn't go ahead with it. There's also Project Fission (bug 1451850) now that sort of subsumes it.
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 3•7 years ago
|
||
Thank you for the info.
We don't need to maintain stacks across threads, as thread
switches always occur when the JS stack is empty.
Would it be possible to root the global object, destroy the JSContext, and
then create a new JSContext on the new thread to use with the global object?
Flags: needinfo?(jdemooij)
Comment 4•7 years ago
|
||
(In reply to Karl Tomlinson (:karlt) from comment #3)
> Would it be possible to root the global object, destroy the JSContext, and
> then create a new JSContext on the new thread to use with the global object?
Not without major JS engine changes. Right now destroying the context would also destroy the runtime and GC everything in it, including your global object.
You'd want the ability to either (a) move/attach a JSRuntime to a different JSContext, or (b) move a JSContext to a different thread. These changes are pretty invasive because they affect core JS engine invariants, so it really should be a last resort.
FWIW, creating a JSContext isn't very fast (a lot of state to initialize) so you probably don't want to do that each time you run a worklet.
Flags: needinfo?(jdemooij)
![]() |
||
Comment 5•7 years ago
|
||
I think the quantum DOM situation is "it's not happening", for what it's worth.
Apart from that, moving objects from one thread to another would have to be a _very_ explicit operation, because it would require moving the object to a different arena/zone/etc. We definitely do not have to worry about things in the same gc arena "living" on different threads, right?
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #5)
> Apart from that, moving objects from one thread to another would have to be
> a _very_ explicit operation, because it would require moving the object to a
> different arena/zone/etc. We definitely do not have to worry about things
> in the same gc arena "living" on different threads, right?
Explicit moving would be doable (from the audio processing perspective).
There would be no need to have any objects "living" on two different threads
concurrently, but all objects associated with the global would need to move
with the global (I can't imagine anything else).
I don't know enough about arenas to know why moving the arena to the new
thread would not be an option.
But I'm reading, between the lines here and elsewhere, the message that
people are very pleased that they no longer need to support a single runtime
on multiple threads, and are taking advantage of that.
I'm not clear on why runtime and context are different elements now, but it
sounds like the message is pretty much "you can't have one without the other".
Thank you for clearing up the current situation with OS threads.
I wanted to confirm that this really was no longer supported in any way,
and I'm hearing a clear no.
I also have last resort options for maintaining a single thread for audio
processing. These are in the realm of moving the audio model back to one
from which we spent many months escaping, but I'm hearing a clear enough
message that I need to consider these.
![]() |
||
Comment 7•7 years ago
|
||
> I don't know enough about arenas to know why moving the arena to the
> new thread would not be an option.
An arena can have objects from multiple different globals in it.
So you can't move an arena. What you can do is create a new arena and move some set of objects to it. This requires a stop-the-world kind of thing so you can update all existing references to those objects to point to the new location. This would need to be the very explicit step.
> people are very pleased that they no longer need to support a single
> runtime on multiple threads,
Yes.... That allows use of niceties like TLS, for example.
What I'm hearing is that you want to do audio processing in a threadpool, but need to run JS as part of audio processing (with audio worklets) and have state that needs to persist across processing switching threads?
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #7)
> > I don't know enough about arenas to know why moving the arena to the
> > new thread would not be an option.
>
> An arena can have objects from multiple different globals in it.
>
> So you can't move an arena. What you can do is create a new arena and move
> some set of objects to it. This requires a stop-the-world kind of thing so
> you can update all existing references to those objects to point to the new
> location. This would need to be the very explicit step.
Updating all existing references sounds expensive.
I'm assuming that the set of threads involving AudioWorklet would use its own
runtime and that all globals in the arena would be associated with that runtime.
If it helps, all use of the same runtime would move threads at the same time,
and so I guess all globals and all objects in the arena would move.
So I assume we don't have to worry about things in the same gc arena living on
different threads, and so I'm not following why the arena can't move.
I haven't yet worked out how cycle-collection works across threads/runtimes,
for a MessageChannel between main thread runtime and worklet thread runtime,
but I'm guessing this extra complication is more or less independent of JS GC.
> What I'm hearing is that you want to do audio processing in a threadpool,
> but need to run JS as part of audio processing (with audio worklets) and
> have state that needs to persist across processing switching threads?
That's essentially the situation.
The need for thread changes isn't just the choice to use a threadpool.
It is the effect of the audio APIs in use owning the threads.
Previously this was more or less dictated by OS APIs, but we are
moving to remoting the audio APIs.
That means that the threads will be in Gecko's control and so the
Gecko-side APIs could be redesigned (assuming this could be negotiated
with the module owners).
![]() |
||
Comment 9•7 years ago
|
||
> all use of the same runtime would move threads at the same time
OK. In that case it should really be "just" a matter of finding all the places where the JS engine uses TLS, I would think... I'll let Jan comment on that, but there's a bunch; TLS is very simple to use for various "global" things.
That's on the JS engine side. On the Gecko side we also use TLS, of course.
> I haven't yet worked out how cycle-collection works across threads/runtimes
It doesn't. Cycle collection is single-threaded. You're right that this is independent of JS.
You could take a set of CCed things that are in the same cycle collector context and move them all across threads as long as TLS use gets found and eliminated and in a debug build the owning thread gets updated (more on this below). But CC can't collect cycles that involve objects on multiple threads.
Comment 10•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #9)
> OK. In that case it should really be "just" a matter of finding all the
> places where the JS engine uses TLS, I would think... I'll let Jan comment
> on that, but there's a bunch
Right, on the JS engine side there are TLS things for JSContext, JitContext, etc. Then there's stuff like stack recursion limits that would have to be reset when we switch a context to a different thread. It's all doable but would definitely need testing infrastructure for the JS shell etc, *if* we want to do this.
Assignee | ||
Comment 11•7 years ago
|
||
Thank you for the discussion. Bug 1473469 is the new plan.
Assignee | ||
Updated•7 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•