[AUDIO_WORKLET] Optimised output buffer copy

[cwoffenden]

A reworking of #22753, which "improves the copy back from the audio worklet's heap to JS by 7-12x depending on the browser." From the previous description:

Since we pass in the stack for the worklet from the caller's heap, its address doesn't change. And since the render quantum size doesn't change after the audio worklet creation, the stack positions for the audio buffers do not change either. This optimisation adds one-time subarray views and replaces the float-by-float copy with a simple set() per channel (per output).

The existing interactive tests (written for the original PR) can be run for comparison:

test/runner interactive.test_audio_worklet_stereo_io
test/runner interactive.test_audio_worklet_2x_stereo_io
test/runner interactive.test_audio_worklet_mono_io
test/runner interactive.test_audio_worklet_2x_hard_pan_io
test/runner interactive.test_audio_worklet_params_mixing
test/runner interactive.test_audio_worklet_memory_growth
test/runner interactive.test_audio_worklet_hard_pans

These test various input/output arrangements as well as parameters (parameters are interesting because, depending on the browser, the sizes change as the params move from static to varying).

The original benchmark of the extracted copy is still valid:

https://wip.numfum.com/cw/2024-10-29/index.html

This is tested with 32- and 64-bit wasm (which required a reordering of how structs and data were stored to avoid alignment issues).

Some explanations:

• Fixed-position output buffer views are created once in theWasmAudioWorkletProcessor constructor
• Stack allocations for the process() call are split into aligned struct data (see the comments) and audio/param data
• The struct writes are simplified by this splitting of data
• ASSERTIONS are used to ensure everything fits and correctly aligns
• The tests account for size changes in the params, which can vary from a single float to 128 floats (a single float nicely showing up any 8-byte alignment issues for wasm64)

Future improvements: the output views are sequential, so instead of of being individual views covering each channel the views could cover one to however-many-views needed, with a single set() being enough for all outputs.

[cwoffenden]

It's not possible to merge the output data copy into a single set() (nor the input data), since the receiving float arrays are independent.

[cwoffenden]

This is done, though I can rebase with all the tests. and merged so the tests are available.

[juj]

The above loop multiplies by * this.bytesPerChannel; outside the loop, though the loop before that multiplies inside the loop. A small micro-opt would be to merge to multiply once outside. (though looks good to me either way)

[cwoffenden]

I'll take a look over this. The most compact would probably be to take the incoming sizes (but I work on the assumption that the browser will do something wrong at some point in the future and throw the maths off).

[cwoffenden]

I took a look and reused the stackMemoryData var (committed). I'm not sure it improves the readability, whereas for outputViewsNeeded it made sense. I'm not sure there are enough channels to warrant it.

[juj]

It looks like we don't ever use this.maxBuffers beyond the call to this.createOutputViews() below?

One way to recoup some of the code size increase would be to avoid storing this.xxx members that aren't needed outsided this scope.

[juj]

Oh I see, we need to know the number of buffers when we call again to this.createOutputViews();.

Maybe then this could do

this.outputViews = new Array(Math.min(((wwParams.stackSize - /*minimum alloc*/ 16) / this.bytesPerChannel) | 0, /*sensible limit*/ 64));

and instead of

this.outputViews.unshift(

inside createOutputViews(), the function would always assign to this.outputViews[i] = , reusing the array with fixed pre-initialized length?

[cwoffenden]

I think this has merit and will result in a saving. Assertions can be against the array length.

I'll try to take a look today.

[cwoffenden]

I factored out tracking the length. Update incoming.

[juj]

LGTM, the logic looks solid now.

I had a couple of minor comments, but those are cosmetic. LGTM either way, this looks good to land to me.

[cwoffenden]

LGTM, the logic looks solid now.

Thanks!

[juj]

One thing that comes to mind that may be worth probing: The TLS section is allocated at the end of the stack, (it should be exclusive of the stack size though).

So you may try to place a global variable __thread int foo = 42; variable in the test test/webaudio/audioworklet_params_mixing.c, and add an assert(foo == 42); to verify that its value never changes during the test. This assert would be inside the audio worklet callback process call.

This will verify that the allocation from stack top won't stomp on TLS variables.

[cwoffenden]

One thing that comes to mind that may be worth probing

[snip]

I can add this as a separate PR, internals like this were one of my worries when starting this last year (most of those worries went away once it was working, but I'm still missing insights like this).

[juj]

Yeah that sounds good.

[cwoffenden]

Changes in as per @juj's suggestions. CI is breaking but it's not related.

[cwoffenden]

Yeah that sounds good.

#25024 tests this.