-
-
Notifications
You must be signed in to change notification settings - Fork 22.4k
Remove postMessage limit on audio position worklet #105541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Remove postMessage limit on audio position worklet #105541
Conversation
It is necessary because the PR that introduced this limit fixed a severe game-breaking bug I'll repeat my suggestion that position reporting should eventually be made opt-in, because in the majority of cases where you play audio you don't care about the playback position. An app that plays a single audio stream is quite a special case. In my project I had a few short sound effects that were repeated a lot, and one background music track, and in this common situation the unlimited position reporting killed performance, making the game unplayable. The review comment on the PR linked above would be worth investigating; maybe there is an alternative solution to the problem. |
@rburing From my understanding of the issue, the root cause was the number of audio position worklets constantly growing, which #102163 aimed to fix by pooling the workers. The I downloaded the MRP from the original issue and built it on 4.4.1.stable, then manually changed the exported I will continue doing some more profiling with audio position disabled, but I suspect this is not a large factor in performance. |
I did some more testing using the MRP (4.4.1.stable web version) on my Windows desktop (Ryzen 9 5900X, GTX 1080, 32 GB RAM). Methodology:
Results from the "use worklet" case:
Results from the "no worklet" case:
Using the worklet on average increases time spent scripting from 10.51% to 10.60%, but this could be within the margin for error. In terms of optimizing for performance, this doesn't seem like a source for meaningful gains, and I would argue that having position functionality disabled would be more confusing devex. Maybe we can get opinions from the audio team? |
This limit was added with the following comment:
I ran some Chrome profiling on my test project, and on my machine (Ryzen 9 5900X, GTX 1080) the
onmessage
callback took 3-5 microseconds to run. Even with CPU throttling at 20x, it only took 0.2ms.The audio JS library on no-thread Web builds posts messages with chunks of sound data, and
audio.worklet.js
also posts messages on every process. Limiting the position message posts doesn't seem to be necessary here.Fixes #105397.