1566369 - Lower the audio thread pool thread count to 1 on Linux

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, enhancement, P2)

Description

[Paul Adenot (:padenot)]

This seem to work well: https://treeherder.mozilla.org/#/jobs?repo=try&revision=814cd62c22892ce6b194cdce01e1f12e38596d54

I've been running with this configuration for some time now, I haven't seen any problems.

This will help lowering the number of RT threads to 2 per content process, and help not reach the limit.

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1566369 - Lower the audioipc thread pool thread count to 1 on Linux. r?kinetik

Comment 2

[Matthew Gregan [:kinetik]]

I'm concerned this will lead to performance issues as we're now going to serialize every cubeb_stream callback on a single thread. It's simple to revert, though, so we can try it.

I'd prefer to retain the ability to use a multi-thread pool here, so if any upstream code starts depending on being called from a single stable thread that will cause problems for planned improvements in AudioIPC.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b2470f9ae71
Lower the audioipc thread pool thread count to 1 on Linux. r=kinetik

Comment 4

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/3b2470f9ae71

Comment 5

[Paul Adenot (:padenot)]

(In reply to Matthew Gregan [:kinetik] from comment #2)

I'm concerned this will lead to performance issues as we're now going to serialize every cubeb_stream callback on a single thread. It's simple to revert, though, so we can try it.

It should be fine. most of the time, those thread are not loaded very high, and the stream events are not exactly happening at the same time, since, down the line, there is a mixer that needs to mix everything and do its job, and it's not running on multiple threads. When/if we support multiple output device, maybe it might be different.

I'd prefer to retain the ability to use a multi-thread pool here, so if any upstream code starts depending on being called from a single stable thread that will cause problems for planned improvements in AudioIPC.

There is the hard requirement that an AudioContext that runs JS or WASM in an AudioWorklet` is always running it on the exact same thread, because SpiderMonkey relies on TLS. We had to come up with https://searchfox.org/mozilla-central/source/dom/media/GraphRunner.cpp#26 because this is not yet the case.

This is however just for a specific AudioContext. Ideally, there would be multiple system level streams in the parent, and multiple threads in each child, and there should be a 1:1 relationship. We could certainly put all the high-latency AudioStream playing to the default device together.

Comment 6

[Andrei Ciure[:aciure]]

Backed out changeset 3b2470f9ae71 (bug 1566369) for causing high frequency failure on test_peerConnection_replaceTrack.html

push that caused the failures: https://hg.mozilla.org/integration/autoland/rev/3b2470f9ae71

failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=257126032&group_state=expanded&fromchange=e17749201d35d24e2f7c12289ec588a657761e61&searchStr=linux%2Cx64%2Cdebug%2Cmochitests%2Cwith%2Csocket%2Cprocess%2Ctest-linux64%2Fdebug-mochitest-media-spi-e10s-2%2Cm-spi%28mda2%29&tochange=37d80eb396bec9cec4504bd52766059636eac3d2

backout: https://hg.mozilla.org/integration/autoland/rev/eb6e49a991d846a99b1c9d536efa8ab391f77dd8

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/eb6e49a991d8

Comment 8

[Paul Adenot (:padenot)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2435c3155510b22574d69a9d9f63b317427a4590 is solid, I'm re-landing this.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/264c44667919
Lower the audioipc thread pool thread count to 1 on Linux. r=kinetik

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/264c44667919