[libwebaudio.js] Convert to handle allocator. NFC

[sbc100]

Also:

• Removing duplication in precondition check.
• Use dbg() for debug messages

The use of the HandleAllocator does add about 70 bytes to the JS payload, but that will be amortized over the different places where HandleAllocator is also uses.

[juj]

that will be amortized over the different places where HandleAllocator is also uses.

Taking a peek at what currently uses HandleAllocator, I see

in e.g. my AudioWorklet MOD player, https://clbri.com/modplayer/ , I'm not sure that it will ever use libfetch, libpromise, libwasmfs_opfs or libwebsocket. So it is not clear that this change will be a win.

The handle allocator itself is a bit contentious. It has a couple of challenges:

1. If after this PR, one does

EMSCRIPTEN_WEBAUDIO_T context1 = emscripten_create_audio_context();
emscripten_destroy_audio_context(context1);

EMSCRIPTEN_WEBAUDIO_T context2 = emscripten_create_audio_context();
emscripten_audio_node_connect(someAudioNode, context1); // Programming error: incorrectly use context1 and not context2

Then the last line will not result in an error, but will silently connect the node to context2 instead. Imagine trying to debug a programming error where a subsystem keeps using a stale audio context in its operation. Accessing an invalid handle can acquire "phantom" meanings in the future.

2. The HandleAllocator is unable to shrink memory, but retains a free list. E.g. temporarily having thousands of allocated handles, and then freeing them, will result in the free list keeping thousands of handles alive. (Imagine a native malloc() that would have a bookkeeping overhead of the max number of allocations ever made) To fix this drawback, the size of the HandleAllocator is likely to grow in the future, diminishing the supposed amortized code size win that is achieved here.

3. Adding dependencies between libraries seems like a pessimization. In general one would aspire to move in the opposite direction - remove dependencies between systems. Whenever the common system is something complex, then the stronger coupling is warranted. But here it is just one JS map/dictionary {} implementation plus tracking a counter that is being shared across systems. That is not much. (and doesn't save on size)

Losing these would not be end of the world, but I am not sure what the upside of centralizing these dependencies is?

[sbc100]

Regarding your concerns (1) and (2), I completely agree. I do think we can and should address those. The nice thing about centralizing this mechanism is that when we do fix either (1) or (2) then all the subsystems that use the handle allocator will benefit. If we have N different allocation system then we cannot systematically address those kinds of issues.

Regarding (3), I'm not sure I agree, emscripten is full of shared abstractions and IMHO they have been a net win. e.g. callUserCallback, withBuiltinMalloc.

I do agree that (1) and (2) need to be address though, so maybe we can address them before landing this change since those concerns apply to all subsystems that use the HandleAllocator, right?

I just though one other nice thing that we can do once all subsystem share a common handle allocator: we can make handles globally unique (at least in debug builds). This was we can prevent even more types of programming errors: where handles from one API are mistakenly passed as handles into another API.. maybe less common but certainly good prevent type confusion.

[sbc100]

Marking this as draft until (1) and (2) can be addressed.

The debugging changes I split out into https://github.com/emscripten-core/emscripten/pull/25655

[juj]

when we do fix either (1) or (2) then all the subsystems that use the handle allocator will benefit.

But if we don't share the code, then the issue that needs benefiting would not even arise.

emscripten is full of shared abstractions and IMHO they have been a net win. e.g. callUserCallback, withBuiltinMalloc.

That is true. I am not arguing that all shared abstractions are bad. But I am arguing that there should be actual substance to the sharing. Here with HandleAllocator, the sharing seems to be about abstracting over a JS {} dictionary, which is very little.

I just though one other nice thing that we can do once all subsystem share a common handle allocator: we can make handles globally unique (at least in debug builds).

It is quite rare for that to cause issues, e.g. passing a Web Audio context handle into a Fetch API call, is something that is really uncommon problem (as opposed to e.g. the use-after-free example above) to occur by a developer in the first place, and the browser JS API endpoints will croak about such issues anyways.