Standard library libc code cannot unconditionally use atomics / tls because of `-sSHARED_MEMORY` setting

[sbc100]

We have a setting called -sSHARED_MEMORY which can be used in programs that don't use either pthreads or wasm workers. In this case we link against the non-threaded version libc and expect all atomic and TLS operations to be lowered away.

My understanding is that this mode is designed user who want to share memory with worker but don't expect to be able use any C/C++ synchronization primitives and instead do all their synchronization in JS.

See https://groups.google.com/g/emscripten-discuss/c/IsRC-MRGQjw for example.

The way this setting is implemented means we cannot currently use _Atomic in libc code. For example attempting to use _Atomic in sbrk yeilds the following error in tests that use -sSHARED_MEMORY:

$ test/runner other.test_shared_memory_minimal_runtime --skip-slow
Test suites:
['test_other']
Running test_other: (1 tests)
(checking sanity from test runner)
shared:INFO: (Emscripten: Running sanity checks)
test_shared_memory_minimal_runtime (test_other.other.test_shared_memory_minimal_runtime) ... Clearing existing test directory
wasm-ld: error: --shared-memory is disallowed by sbrk.o because it was not compiled with 'atomics' or 'bulk-memory' features.
emcc: error: '/usr/local/google/home/sbc/dev/wasm/llvm-build/bin/wasm-ld -o shared_memory.wasm /tmp/emscripten_temp_j6d0bcpl/shared_memory_0.o -L/usr/local/google/home/sbc/dev/wasm/emscripten/cache/sysroot/lib/wasm32-emscripten /usr/local/google/home/sbc/dev/wasm/emscripten/cache/sysroot/lib/wasm32-emscripten/crtbegin.o -lGL -lal -lhtml5 -lbulkmemory -lstubs-debug -lnoexit -lc-debug -ldlmalloc-noerrno -lcompiler_rt -lsockets -mllvm -combiner-global-alias-analysis=false -mllvm -enable-emscripten-sjlj -mllvm -disable-lsr /tmp/tmp26_xe79ylibemscripten_js_symbols.so --import-memory --shared-memory --strip-debug --export-if-defined=main --export-if-defined=__start_em_asm --export-if-defined=__stop_em_asm --export-if-defined=__start_em_lib_deps --export-if-defined=__stop_em_lib_deps --export-if-defined=__start_em_js --export-if-defined=__stop_em_js --export-if-defined=__main_argc_argv --export-if-defined=fflush --export=emscripten_stack_get_end --export=emscripten_stack_get_free --export=emscripten_stack_get_base --export=emscripten_stack_get_current --export=emscripten_stack_init --export=stackSave --export=stackRestore --export=stackAlloc --export=__get_temp_ret --export=__set_temp_ret --export=__wasm_call_ctors --export-table -z stack-size=65536 --initial-memory=16777216 --max-memory=16777216 --no-entry --stack-first --table-base=1' failed (returned 1)

See #20793.

Possible solutions:

1. Link against libc-mt when building with -sSHARED_MEMORY
2. Tell the linker to ignore "not compiled with 'atomics' or 'bulk-memory' features." errors when linking with -sSHARED_MEMORY
3. Remove -sSHARED_MEMORY as a user-facing flag and ask users to use either pthreads or wasm workers if they want shared memory.

[sbc100]

@tlively @kripken @juj @caiiiycuk

[tlively]

Solution (3) would be good if we can get away with it. Exposing shared memory for users to "do their own thing" without the compiler being aware of it seems like a recipe for trouble.

If we can't remove -sSHARED_MEMORY, then (1) seems like the clear winner.

[sbc100]

Perhaps for backwards compat with could have the command line -sSHARED_MEMORY (which should have very few users) imply -pthread.. but that might be more complexity that its worth

[caiiiycuk]

My case was as follows: I have a C++ program (DOSBox, referred to as the backend) that generates an RGB frame every 16ms (60FPS). Then, I pass this RGB frame to JavaScript for rendering.

The backend can operate in both worker or main thread modes, determined by the bootstrap JavaScript script. The WebAssembly (wasm) module remains the same for both direct and worker modes. When the backend operates in the worker mode, each frame is sent to the main thread in this manner:

// worker
emulation()
EM_ASM((
  const rgb = Module.HEAPU8.slice(ptr, ptr + width * height * 3);
  postMessage({ rgb }, [rgb.buffer]); 
), ptr, width, height);

What I dislike here: I need to create a copy of the RGB frame, and in the case of slow processing (frame skip), it will result in additional memory consumption by the browser. My idea is, what if I enable SHARED_MEMORY, then I can send only a message stating the need to update a frame without data, and directly read the RGB buffer from shared memory, like:

emulation()
EM_ASM((
  postMessage({ ptr, width, height });
), ptr, width, height);

This approach will decrease the emulation's memory consumption and prevent an additional copy of memory in the event of a frame skip. However, under normal execution, it may have the same speed because gl.updateTexture will require the same Module.HEAPU8.slice to update the texture (still one copy per frame).

Enabling SharedArrayBuffer can be problematic for deployment since most elements like YouTube and ads won't function properly on a website with SharedArrayBuffer support enabled. It's not worth the trouble. For me, this is no longer a viable option.

[juj]

I think overall to be able to clean up a few lines of code from sbrk.c might not yet be a rationale enough to drop supporting -sSHARED_MEMORY.

Originally I thought that use case was a bit weird, but the above description by caiiiycuk does make sense, there is no other way to solve that use case.

(on that last note, I do wish that COOP, COEP and CORP headers weren't necessary. They are difficult to understand and use, Unity game developers communicate that often. But I don't know of any other way either)

[sbc100]

I think the use case above can just as well be enabled using -pthread. Assuming the you don't actually call any pthread functions that code should be very minimal.

Maintain and extra -sSHARED_MEMORY build mode is not just about sbrk.c, it about the maintenance and testing of yet another build mode. My proposal is that we remove it, but only if we can show the cost using -pthread for this purpose is very low (both in code size and in performance).

[juj]

it's about the maintenance and testing of yet another build mode

I recall there was some review churn with audio_worklets + wasm_workers vs shared_memory flags in a past PR, but so far I think that it hasn't been overwhelming.

but only if we can show the cost using -pthread for this purpose is very low (both in code size and in performance).

Agree, that's a good approach. I wonder originally when @azakai asked me to add that mode, was it motivated by @caiiiycuk 's use case? Or was there some other user? I forget who was driving the need at the time.

[kripken]

I think the motivation for SHARED_MEMORY was for use cases like that one that was described, yeah.

Also agreed that it is sad that COOP/COEP/etc. are making it hard to use SharedArrayBuffer these days...

Given that COOP/COEP difficulty is the same with SHARED_MEMORY and pthreads, I'd be ok with making SHARED_MEMORY an internal setting if the overhead isn't high. As a quick test I tried ./emcc test/hello_libcxx.cpp -O3. pthreads adds 10% to code size and wasm workers adds 1.5%. So suggesting people use one of those seems viable, and easier to maintain.