Tonejs / Tone.js

A Web Audio framework for making interactive music in the browser.
https://tonejs.github.io
MIT License
13.27k stars 964 forks source link

DetectCycles Performance Overhead #1196

Closed abswiz closed 2 months ago

abswiz commented 1 year ago

Describe the bug

There appears to be a recursive loop in method detectCyles, part of the standardized-audio-context, often resulting in this routine consuming over 80% of the CPU and the audio rendering window being frequently missed. More info is available on the following link, raised on the standarsized-audio-context git page.

https://github.com/chrisguttandin/standardized-audio-context/issues/996

This overhead appears to get exponentially complex as the size of the audio graph increases.

To Reproduce

We think this is due to the massive size of the audio graphs that are produced by some synths, fx units, etc ... the metal synth for example, especially where they are wrapped by PolySynth.

To reproduce, you can register at dappledark.com, start the Composer and record the performance in dev tools. We have implemented significant countermeasures, including various pooling mechanisms, to work around this issue, but would appreciate any insights or improvements in this area. We are currently using v14.8.40 and cannot locate any release information beyond July 2020, although there are CDNJS entries, post v14.7.29. is there a location where new features and improvements are listed post July 2020?

Expected behavior The performance should not degrade to such an extent with a single point appearing to be a significant bottleneck.

What I've tried Please see earlier info.

Additional context Please see link in the bug description.

marcelblum commented 1 year ago

I think it would be really helpful to know what @chrisguttandin means by this secretive "bug 164" 🤯, surely not this one, maybe the project has some internal bug tracker he's referring to? I see the detectCycles function hasn't been touched in over 3 years.

I noticed that your project already lists Chrome as a recommended requirement. As Chrome(ium) has long had the best and most complete implementation of the Web Audio API, here in mid 2023, for users on latest Chrome/Chromium derivatives, I'd argue there is little need for standardized-audio-context, save for maybe some extremely edge cases. I think you'd be well served by doing browser detection and not using standardized-audio-context if Blink is found as that should avoid the issue and improve performance for the majority of your users (and hopefully not create any new ones, obviously you'd need to rigorously test, maybe Chris can speak more on such edge cases).

I use Tone.js primarily in Electron apps where I don't need to use standardized-audio-context because Chromium, and haven't run into any major issues without it AFAICT - in fact it certainly reduces overhead to avoid using it - and over the years I've contributed several PRs to make Tone work better when standardized-audio-context is not in use as there were a few places in the codebase that assumed use of standardized-audio-context which needed to be modded.

(Not arguing for Tone.js to ditch standardized-audio-context as it brings conveniences, just noting that for certain complex/advanced use cases where there are obvious performance benefits, there's a good argument for browser detecting and avoiding use of it when Chromium).

abswiz commented 1 year ago

Thanks Marcel ... appreciate the info. We already perform some feature detection for midi and latency hint support, as well as iOS audio activation. Although we certify Chrome, we do support a wide range of browsers, including tablets. The Composer is a responsive application. We are undergoing some performance optimisation work and wanted to explore any enhancements in this area, before upgrading the Tone JS version, but we cannot locate any release info since v14.7.29.

Rgds Abs

marcelblum commented 1 year ago

You're not going to find any major updates since that version, mostly just little bug fixes, edge case improvements, minor optimizations, deps updates (including standardized-audio-context). You can browse the dev branch commit history to get an idea.

abswiz commented 1 year ago

Thanks again Marcel ... you've been most helpful ... appreciate your insight.

tambien commented 2 months ago

Closing this here, since it seems like it is mostly about https://github.com/chrisguttandin/standardized-audio-context/issues/996 and not Tone.js.

We can update standardized-audio-context if/when there's an update on this