From a18d699ac58b192ebed1219e3caf3949cf0dd07a Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 9 May 2025 23:07:37 +0100 Subject: [PATCH 01/14] Add codesize for pthread + memory growth --- ...odesize_minimal_pthreads_memgrowth.exports | 17 ++++ ..._codesize_minimal_pthreads_memgrowth.funcs | 91 +++++++++++++++++++ ...codesize_minimal_pthreads_memgrowth.gzsize | 1 + ...odesize_minimal_pthreads_memgrowth.imports | 15 +++ ...codesize_minimal_pthreads_memgrowth.jssize | 1 + ...t_codesize_minimal_pthreads_memgrowth.sent | 15 +++ ...t_codesize_minimal_pthreads_memgrowth.size | 1 + test/test_other.py | 8 +- 8 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.exports create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.funcs create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.imports create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.sent create mode 100644 test/other/codesize/test_codesize_minimal_pthreads_memgrowth.size diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.exports b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.exports new file mode 100644 index 0000000000000..0b1b4cd55866e --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.exports @@ -0,0 +1,17 @@ +A (_emscripten_thread_exit) +B (_emscripten_check_mailbox) +C (emscripten_stack_set_limits) +D (_emscripten_stack_restore) +E (_emscripten_stack_alloc) +F (emscripten_stack_get_current) +p (__wasm_call_ctors) +q (add) +r (main) +s (__indirect_function_table) +t (_emscripten_tls_init) +u (pthread_self) +v (_emscripten_proxy_main) +w (_emscripten_thread_init) +x (_emscripten_thread_crashed) +y (_emscripten_run_on_main_thread_js) +z (_emscripten_thread_free_data) diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.funcs b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.funcs new file mode 100644 index 0000000000000..c6b56fc361d82 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.funcs @@ -0,0 +1,91 @@ +$__emscripten_stdout_seek +$__errno_location +$__memcpy +$__memset +$__pthread_getspecific +$__pthread_mutex_lock +$__pthread_mutex_trylock +$__pthread_mutex_unlock +$__pthread_rwlock_timedrdlock +$__pthread_rwlock_tryrdlock +$__pthread_rwlock_trywrlock +$__pthread_rwlock_unlock +$__pthread_self_internal +$__pthread_setcancelstate +$__set_thread_state +$__stdio_write +$__timedwait +$__timedwait_cp +$__tl_lock +$__tl_unlock +$__wait +$__wake +$__wake +$__wasi_syscall_ret +$__wasm_call_ctors +$__wasm_init_memory +$__wasm_init_tls +$_emscripten_check_mailbox +$_emscripten_proxy_main +$_emscripten_run_on_main_thread_js +$_emscripten_stack_alloc +$_emscripten_stack_restore +$_emscripten_thread_crashed +$_emscripten_thread_exit +$_emscripten_thread_free_data +$_emscripten_thread_init +$_emscripten_thread_mailbox_init +$_emscripten_tls_init +$_emscripten_yield +$_main_thread +$a_cas +$a_cas +$a_cas_p +$a_dec +$a_fetch_add +$a_fetch_add +$a_inc +$a_store +$a_swap +$add +$call_callback_then_free_ctx +$call_cancel_then_free_ctx +$call_then_finish_task +$call_with_ctx +$cancel_active_ctxs +$cancel_ctx +$cancel_notification +$dispose_chunk +$do_proxy +$em_proxying_ctx_deinit +$em_task_queue_cancel +$em_task_queue_create +$em_task_queue_dequeue +$em_task_queue_enqueue +$em_task_queue_execute +$em_task_queue_free +$em_task_queue_is_empty +$emscripten_builtin_free +$emscripten_builtin_malloc +$emscripten_futex_wait +$emscripten_futex_wake +$emscripten_stack_get_current +$emscripten_stack_set_limits +$free_ctx +$get_tasks_for_thread +$init_active_ctxs +$init_file_lock +$init_mparams +$lock +$main +$nodtor +$pthread_attr_destroy +$pthread_cond_signal +$pthread_mutex_destroy +$pthread_setspecific +$receive_notification +$remove_active_ctx +$run_js_func +$sbrk +$undo +$unlock diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize new file mode 100644 index 0000000000000..31a1f807b4325 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize @@ -0,0 +1 @@ +4218 diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.imports b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.imports new file mode 100644 index 0000000000000..8fb7da4f6b7e7 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.imports @@ -0,0 +1,15 @@ +a (memory) +b (emscripten_get_now) +c (exit) +d (_emscripten_thread_set_strongref) +e (fd_write) +f (emscripten_runtime_keepalive_check) +g (emscripten_resize_heap) +h (emscripten_exit_with_live_runtime) +i (emscripten_check_blocking_allowed) +j (_emscripten_thread_mailbox_await) +k (_emscripten_thread_cleanup) +l (_emscripten_receive_on_main_thread_js) +m (_emscripten_notify_mailbox_postmessage) +n (_emscripten_init_main_thread_js) +o (__pthread_create_js) diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize new file mode 100644 index 0000000000000..7684594db1880 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize @@ -0,0 +1 @@ +8838 diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.sent b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.sent new file mode 100644 index 0000000000000..8fb7da4f6b7e7 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.sent @@ -0,0 +1,15 @@ +a (memory) +b (emscripten_get_now) +c (exit) +d (_emscripten_thread_set_strongref) +e (fd_write) +f (emscripten_runtime_keepalive_check) +g (emscripten_resize_heap) +h (emscripten_exit_with_live_runtime) +i (emscripten_check_blocking_allowed) +j (_emscripten_thread_mailbox_await) +k (_emscripten_thread_cleanup) +l (_emscripten_receive_on_main_thread_js) +m (_emscripten_notify_mailbox_postmessage) +n (_emscripten_init_main_thread_js) +o (__pthread_create_js) diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.size b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.size new file mode 100644 index 0000000000000..be9d770d06316 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.size @@ -0,0 +1 @@ +19397 diff --git a/test/test_other.py b/test/test_other.py index c196254cf1422..5a04cfff1af5f 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -9294,8 +9294,12 @@ def test_codesize_minimal(self, *args): self.run_codesize_test('minimal.c', *args) @node_pthreads - def test_codesize_minimal_pthreads(self): - self.run_codesize_test('minimal_main.c', ['-Oz', '-pthread', '-sPROXY_TO_PTHREAD', '-sSTRICT']) + @parameterized({ + '': ([],), + 'memgrowth': (['-sALLOW_MEMORY_GROWTH'],), + }) + def test_codesize_minimal_pthreads(self, args): + self.run_codesize_test('minimal_main.c', ['-Oz', '-pthread', '-sPROXY_TO_PTHREAD', '-sSTRICT'] + args) @parameterized({ 'noexcept': (['-O2'], [], ['waka']), # noqa From d7962d99f929cce678c08fc6d2f0ec786af2fc9a Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 9 May 2025 23:22:13 +0100 Subject: [PATCH 02/14] Simplify GROWABLE_HEAP transform Similar to #24291, but for the transform handling `-pthread` + `-sALLOW_MEMORY_GROWTH`. In the first commit I added a codesize variation to commit current sizes, and in the second you can see that this transform actually saves the total size in addition to making acorn-optimizer simpler. --- site/source/docs/porting/pthreads.rst | 2 +- src/growableHeap.js | 60 +------------------ src/modules.mjs | 11 +--- ...codesize_minimal_pthreads_memgrowth.gzsize | 2 +- ...codesize_minimal_pthreads_memgrowth.jssize | 2 +- test/test_other.py | 4 +- tools/acorn-optimizer.mjs | 52 ++-------------- 7 files changed, 14 insertions(+), 119 deletions(-) diff --git a/site/source/docs/porting/pthreads.rst b/site/source/docs/porting/pthreads.rst index 6f00b3971d6c3..a8507a19b8927 100644 --- a/site/source/docs/porting/pthreads.rst +++ b/site/source/docs/porting/pthreads.rst @@ -144,7 +144,7 @@ The Emscripten implementation for the pthreads API should follow the POSIX stand - Note that the function emscripten_num_logical_cores() will always return the value of navigator.hardwareConcurrency, i.e. the number of logical cores on the system, even when shared memory is not supported. This means that it is possible for emscripten_num_logical_cores() to return a value greater than 1, while at the same time emscripten_has_threading_support() can return false. The return value of emscripten_has_threading_support() denotes whether the browser has shared memory support available. -- Pthreads + memory growth (``ALLOW_MEMORY_GROWTH``) is especially tricky, see `Wasm design issue #1271 `_. This currently causes JS accessing the Wasm memory to be slow - but this will likely only be noticeable if the JS does large amounts of memory reads and writes (Wasm runs at full speed, so moving work over can fix this). This also requires that your JS be aware that the HEAP* views may need to be updated - JS code embedded with ``--js-library`` etc will automatically be transformed to use the ``GROWABLE_HEAP_*`` helper functions where ``HEAP*`` are used, but external code that uses ``Module.HEAP*`` directly may encounter problems with views being smaller than memory. +- Pthreads + memory growth (``ALLOW_MEMORY_GROWTH``) is especially tricky, see `Wasm design issue #1271 `_. This currently causes JS accessing the Wasm memory to be slow - but this will likely only be noticeable if the JS does large amounts of memory reads and writes (Wasm runs at full speed, so moving work over can fix this). This also requires that your JS be aware that the HEAP* views may need to be updated - JS code embedded with ``--js-library`` etc will automatically be transformed to use the ``GROWABLE_HEAP`` helper function, but external code that uses ``Module.HEAP*`` directly may encounter problems with views being smaller than memory. .. _Allocator_performance: diff --git a/src/growableHeap.js b/src/growableHeap.js index 099df836d0768..abfe289bbd0be 100644 --- a/src/growableHeap.js +++ b/src/growableHeap.js @@ -6,63 +6,9 @@ // Support for growable heap + pthreads, where the buffer may change, so JS views // must be updated. -function GROWABLE_HEAP_I8() { - if (wasmMemory.buffer != HEAP8.buffer) { +function GROWABLE_HEAP(arr) { + if (wasmMemory.buffer != arr.buffer) { updateMemoryViews(); } - return HEAP8; -} -function GROWABLE_HEAP_U8() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAPU8; -} -function GROWABLE_HEAP_I16() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAP16; -} -function GROWABLE_HEAP_U16() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAPU16; -} -function GROWABLE_HEAP_I32() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAP32; -} -function GROWABLE_HEAP_U32() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAPU32; -} -function GROWABLE_HEAP_I64() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAP64; -} -function GROWABLE_HEAP_U64() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAPU64; -} -function GROWABLE_HEAP_F32() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAPF32; -} -function GROWABLE_HEAP_F64() { - if (wasmMemory.buffer != HEAP8.buffer) { - updateMemoryViews(); - } - return HEAPF64; + return arr; } diff --git a/src/modules.mjs b/src/modules.mjs index ef59b326d50d2..5f198cdf727fd 100644 --- a/src/modules.mjs +++ b/src/modules.mjs @@ -457,16 +457,7 @@ function exportRuntimeSymbols() { if (PTHREADS && ALLOW_MEMORY_GROWTH) { runtimeElements.push( - 'GROWABLE_HEAP_I8', - 'GROWABLE_HEAP_U8', - 'GROWABLE_HEAP_I16', - 'GROWABLE_HEAP_U16', - 'GROWABLE_HEAP_I32', - 'GROWABLE_HEAP_U32', - 'GROWABLE_HEAP_I64', - 'GROWABLE_HEAP_U64', - 'GROWABLE_HEAP_F32', - 'GROWABLE_HEAP_F64', + 'GROWABLE_HEAP', ); } if (USE_OFFSET_CONVERTER) { diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize index 31a1f807b4325..8597d55482f06 100644 --- a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize @@ -1 +1 @@ -4218 +4181 diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize index 7684594db1880..5df4b65f6340c 100644 --- a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize @@ -1 +1 @@ -8838 +8654 diff --git a/test/test_other.py b/test/test_other.py index 5a04cfff1af5f..4d4cd390abd26 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -10544,9 +10544,9 @@ def test_pthreads_growth_and_unsigned(self): '-sMAXIMUM_MEMORY=4GB', '-sALLOW_MEMORY_GROWTH']) # growable-heap must not interfere with heap unsigning, and vice versa: # we must have both applied, that is - # - GROWABLE_HEAP_I8() replaces HEAP8 + # - GROWABLE_HEAP() wraps HEAP8 # - $0 gets an >>> 0 unsigning - self.assertContained('GROWABLE_HEAP_I8().set([ 1, 2, 3 ], $0 >>> 0)', + self.assertContained('GROWABLE_HEAP(HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', read_file('a.out.js')) @parameterized({ diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 4a47979e3feed..f1b80b560bce8 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1288,17 +1288,17 @@ function littleEndianHeap(ast) { }); } -// Instrument heap accesses to call GROWABLE_HEAP_* helper functions instead, which allows +// Instrument heap accesses to call GROWABLE_HEAP helper function, which allows // pthreads + memory growth to work (we check if the memory was grown on another thread // in each access), see #8365. function growableHeap(ast) { recursiveWalk(ast, { FunctionDeclaration(node, c) { - // Do not recurse into to GROWABLE_HEAP_ helper functions themselves. + // Do not recurse into to GROWABLE_HEAP helper function itself. if ( !( node.id.type === 'Identifier' && - (node.id.name.startsWith('GROWABLE_HEAP_') || node.id.name === 'LE_HEAP_UPDATE') + (node.id.name === 'GROWABLE_HEAP' || node.id.name === 'LE_HEAP_UPDATE') ) ) { c(node.body); @@ -1323,50 +1323,8 @@ function growableHeap(ast) { }); }, Identifier: (node) => { - if (node.name.startsWith('HEAP')) { - // Turn HEAP8 into GROWABLE_HEAP_I8() etc - switch (node.name) { - case 'HEAP8': { - makeCallExpression(node, 'GROWABLE_HEAP_I8', []); - break; - } - case 'HEAPU8': { - makeCallExpression(node, 'GROWABLE_HEAP_U8', []); - break; - } - case 'HEAP16': { - makeCallExpression(node, 'GROWABLE_HEAP_I16', []); - break; - } - case 'HEAPU16': { - makeCallExpression(node, 'GROWABLE_HEAP_U16', []); - break; - } - case 'HEAP32': { - makeCallExpression(node, 'GROWABLE_HEAP_I32', []); - break; - } - case 'HEAPU32': { - makeCallExpression(node, 'GROWABLE_HEAP_U32', []); - break; - } - case 'HEAP64': { - makeCallExpression(node, 'GROWABLE_HEAP_I64', []); - break; - } - case 'HEAPU64': { - makeCallExpression(node, 'GROWABLE_HEAP_U64', []); - break; - } - case 'HEAPF32': { - makeCallExpression(node, 'GROWABLE_HEAP_F32', []); - break; - } - case 'HEAPF64': { - makeCallExpression(node, 'GROWABLE_HEAP_F64', []); - break; - } - } + if (isEmscriptenHEAP(node.name)) { + makeCallExpression(node, 'GROWABLE_HEAP', [{...node}]); } }, }); From e87c034b330e862e9a156efdedb08bdc4cb89d6c Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 9 May 2025 23:45:58 +0100 Subject: [PATCH 03/14] fmt --- src/modules.mjs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/modules.mjs b/src/modules.mjs index 5f198cdf727fd..62e19913e039b 100644 --- a/src/modules.mjs +++ b/src/modules.mjs @@ -456,9 +456,7 @@ function exportRuntimeSymbols() { } if (PTHREADS && ALLOW_MEMORY_GROWTH) { - runtimeElements.push( - 'GROWABLE_HEAP', - ); + runtimeElements.push('GROWABLE_HEAP'); } if (USE_OFFSET_CONVERTER) { runtimeElements.push('WasmOffsetConverter'); From e44cfdea64ede4c80cbfaa84b389ed633919ddca Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Sat, 10 May 2025 00:35:39 +0100 Subject: [PATCH 04/14] Rebaseline --- test/js_optimizer/growableHeap-output.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/js_optimizer/growableHeap-output.js b/test/js_optimizer/growableHeap-output.js index 0e70a486b170c..8a7f1b4a68ae8 100644 --- a/test/js_optimizer/growableHeap-output.js +++ b/test/js_optimizer/growableHeap-output.js @@ -1,13 +1,13 @@ function f() { var $2 = 0; - GROWABLE_HEAP_I32()[$0 >> 2] = $2 + 1; - $9 = GROWABLE_HEAP_U8()[$2 >> 0] | 0; - +GROWABLE_HEAP_F64()[x >> 3]; - GROWABLE_HEAP_I64()[x >> 3] = GROWABLE_HEAP_I64()[y >> 3]; + GROWABLE_HEAP(HEAP32)[$0 >> 2] = $2 + 1; + $9 = GROWABLE_HEAP(HEAPU8)[$2 >> 0] | 0; + +GROWABLE_HEAP(HEAPF64)[x >> 3]; + GROWABLE_HEAP(HEAP64)[x >> 3] = GROWABLE_HEAP(HEAP64)[y >> 3]; } function libraryFunc(ptr, val) { - if (ptr < GROWABLE_HEAP_I8().length) { - Atomics.wait(GROWABLE_HEAP_I32(), ptr, val); + if (ptr < GROWABLE_HEAP(HEAP8).length) { + Atomics.wait(GROWABLE_HEAP(HEAP32), ptr, val); } } From bc5a82f9d4db789fc45811606b308b56154bc5e2 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Tue, 13 May 2025 18:59:59 +0100 Subject: [PATCH 05/14] Remove GROWABLE_HEAP from runtimeElements as per review --- src/modules.mjs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/modules.mjs b/src/modules.mjs index 62e19913e039b..24eb9dd2d63e3 100644 --- a/src/modules.mjs +++ b/src/modules.mjs @@ -455,9 +455,6 @@ function exportRuntimeSymbols() { runtimeElements.push('HEAP_DATA_VIEW'); } - if (PTHREADS && ALLOW_MEMORY_GROWTH) { - runtimeElements.push('GROWABLE_HEAP'); - } if (USE_OFFSET_CONVERTER) { runtimeElements.push('WasmOffsetConverter'); } From 20e052c7140e9c36d44851c84dd3384a045a1e72 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 14 May 2025 15:46:28 +0100 Subject: [PATCH 06/14] Remove forcible updateMemoryViews in establishStackSpace Memory views are automatically and lazily updated by the HEAPxx[...] transform, so this unconditional transform is not necessary. --- src/lib/libpthread.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/lib/libpthread.js b/src/lib/libpthread.js index 28910796a3648..fd2a72de0a31f 100644 --- a/src/lib/libpthread.js +++ b/src/lib/libpthread.js @@ -979,11 +979,6 @@ var LibraryPThread = { $establishStackSpace__internal: true, $establishStackSpace__deps: ['$stackRestore', 'emscripten_stack_set_limits'], $establishStackSpace: (pthread_ptr) => { -#if ALLOW_MEMORY_GROWTH - // If memory growth is enabled, the memory views may have gotten out of date, - // so resync them before accessing the pthread ptr below. - updateMemoryViews(); -#endif var stackHigh = {{{ makeGetValue('pthread_ptr', C_STRUCTS.pthread.stack, '*') }}}; var stackSize = {{{ makeGetValue('pthread_ptr', C_STRUCTS.pthread.stack_size, '*') }}}; var stackLow = stackHigh - stackSize; From dc93fa4754ef8dbcf7d185d683a7fc7ea601ec39 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 14 May 2025 15:51:49 +0100 Subject: [PATCH 07/14] Make pthread growth tests more rigorous against racy heap updates --- test/pthread/test_pthread_memory_growth.c | 59 ++++++---- .../test_pthread_memory_growth_mainthread.c | 101 +++++++++++++----- test/test_browser.py | 18 ++-- 3 files changed, 120 insertions(+), 58 deletions(-) diff --git a/test/pthread/test_pthread_memory_growth.c b/test/pthread/test_pthread_memory_growth.c index ce9e370d214d5..2b8d9486d868d 100644 --- a/test/pthread/test_pthread_memory_growth.c +++ b/test/pthread/test_pthread_memory_growth.c @@ -4,42 +4,61 @@ // found in the LICENSE file. #include -#include -#include #include #include -#include +#include + +// We want to test that JavaScript access in the main thread automatically updates the memory views if the heap has grown from the pthread meanwhile. +// +// Checking this correctly is somewhat tricky because a lot of Emscripten APIs access heap on their own, so the memory views might be updated +// by them before we explicitly check the state in our own JS routines below, which leads to false positives (test passing even though in +// isolation the heap access in our own JS is not the one updating the memory views). +// +// To test it in isolation, we need to use only EM_JS - which is as close to a pure JS call as we can get - and not EM_ASM, pthread_join, proxying +// etc., not even `puts`/`printf` for logging - as all of those access the heap from JS on their own and might update the memory views too early. +// Not using standard proxying mechanisms also means we need to drop down all the way to raw atomics. + +EM_JS(void, assert_initial_heap_state, (), { + console.log(`Checking initial heap state on the ${ENVIRONMENT_IS_PTHREAD ? 'worker' : 'main'} thread`); + assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); +}); + +EM_JS(void, assert_final_heap_state, (const char* buffer, int finalHeapSize), { + assert(HEAP8.length > finalHeapSize, "end with >64MB"); + assert(HEAP8[buffer] === 42, "readable from JS"); +}); + +#define FINAL_HEAP_SIZE (64 * 1024 * 1024) static void *thread_start(void *arg) { + assert_initial_heap_state(); // allocate more memory than we currently have, forcing a growth - printf("thread_start\n"); - char* buffer = (char*)malloc(64 * 1024 * 1024); + char* buffer = malloc(FINAL_HEAP_SIZE); assert(buffer); + // Write value at the end of the buffer to check that any thread can access addresses beyond the initial heap size. + buffer += FINAL_HEAP_SIZE - 1; *buffer = 42; - pthread_exit((void*)buffer); + *(const char *_Atomic *)arg = buffer; + return NULL; } int main() { - printf("prep\n"); + assert_initial_heap_state(); pthread_t thr; + const char *_Atomic buffer = NULL; + int res = pthread_create(&thr, NULL, thread_start, &buffer); + assert(res == 0); - printf("start\n"); - EM_ASM({ assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB") }); + while (!buffer); - printf("create\n"); - int s = pthread_create(&thr, NULL, thread_start, (void*)NULL); - assert(s == 0); - void* result = NULL; - - printf("join\n"); - s = pthread_join(thr, &result); - assert(result != 0); // allocation should have succeeded - char* buffer = (char*)result; assert(*buffer == 42); // should see the value the thread wrote - EM_ASM({ assert(HEAP8.length > 64 * 1024 * 1024, "end with >64MB") }); + assert_final_heap_state(buffer, FINAL_HEAP_SIZE); + + res = pthread_join(thr, NULL); + assert(res == 0); + return 0; } - diff --git a/test/pthread/test_pthread_memory_growth_mainthread.c b/test/pthread/test_pthread_memory_growth_mainthread.c index bc0e602b09cdb..0bc1dd2645b1d 100644 --- a/test/pthread/test_pthread_memory_growth_mainthread.c +++ b/test/pthread/test_pthread_memory_growth_mainthread.c @@ -4,51 +4,94 @@ // found in the LICENSE file. #include -#include -#include #include #include -#include +#include -static void *thread_start(void *arg) +// We want to test that JavaScript access in a pthread automatically updates the memory views if the heap has grown on the main thread meanwhile. +// +// Checking this correctly is somewhat tricky because a lot of Emscripten APIs access heap on their own, so the memory views might be updated +// by them before we explicitly check the state in our own JS routines below, which leads to false positives (test passing even though in +// isolation the heap access in our own JS is not the one updating the memory views). +// +// To test it in isolation, we need to use only EM_JS - which is as close to a pure JS call as we can get - and not EM_ASM, pthread_join, proxying +// etc., not even `puts`/`printf` for logging - as all of those access the heap from JS on their own and might update the memory views too early. +// Not using standard proxying mechanisms also means we need to drop down all the way to raw atomics. + +EM_JS(void, js_assert_initial_heap_state, (), { + console.log(`Checking initial heap state on the ${ENVIRONMENT_IS_PTHREAD ? 'worker' : 'main'} thread`); + assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); +}); + +EM_JS(void, js_assert_final_heap_state, (const char* buffer, int finalHeapSize), { + console.log(`Checking final heap state on the ${ENVIRONMENT_IS_PTHREAD ? 'worker' : 'main'} thread`); + assert(HEAP8.length > finalHeapSize, "end with >64MB"); + assert(HEAP8[buffer] === 42, "readable from JS"); +}); + +_Atomic enum state { + INITIAL_STATE, + THREAD_CHECKED_INITIAL_STATE, + FINAL_STATE +} state; + +const char *_Atomic buffer; + +static void assert_initial_heap_state() { - char* buffer = (char*)arg; - assert(buffer); + assert(state == INITIAL_STATE); + assert(buffer == NULL); + js_assert_initial_heap_state(); +} + +#define FINAL_HEAP_SIZE (64 * 1024 * 1024) + +static void assert_final_heap_state() +{ + assert(state == FINAL_STATE); + assert(buffer != NULL); assert(*buffer == 42); - EM_ASM({ assert(HEAP8[$0] === 42, "readable from JS in worker") }, buffer); - pthread_exit((void*)43); + js_assert_final_heap_state(buffer, FINAL_HEAP_SIZE); } -char* test() { - // allocate more memory than we currently have, forcing a growth - char* buffer = (char*)malloc(64 * 1024 * 1024); +static void *thread_start(void *arg) +{ + assert_initial_heap_state(); + // Tell main thread that we checked the initial state and it can allocate. + state = THREAD_CHECKED_INITIAL_STATE; + // Wait for the main thread to allocate memory. + while (state != FINAL_STATE); + // Check the final heap state. + assert_final_heap_state(); + return NULL; +} + +static char *alloc_beyond_initial_heap() +{ + char *buffer = malloc(FINAL_HEAP_SIZE); assert(buffer); + // Write value at the end of the buffer to check that any thread can access addresses beyond the initial heap size. + buffer += FINAL_HEAP_SIZE - 1; *buffer = 42; return buffer; } int main() { - printf("prep\n"); - - printf("start main\n"); - EM_ASM({ assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB") }); - char* buffer = test(); - assert(*buffer == 42); // should see the value the code wrote - EM_ASM({ assert(HEAP8[$0] === 42, "readable from JS") }, buffer); - EM_ASM({ assert(HEAP8.length > 64 * 1024 * 1024, "end with >64MB") }); - - printf("start thread\n"); pthread_t thr; - int s = pthread_create(&thr, NULL, thread_start, (void*)buffer); - assert(s == 0); - printf("join\n"); - void* result = NULL; - s = pthread_join(thr, &result); - assert(result == (void*)43); + int res = pthread_create(&thr, NULL, thread_start, NULL); + assert(res == 0); + // Check initial state in both threads before allocating more memory. + assert_initial_heap_state(); + while (state != THREAD_CHECKED_INITIAL_STATE); - printf("finish\n"); + // allocate more memory than we currently have, forcing a growth + buffer = alloc_beyond_initial_heap(); + state = FINAL_STATE; + + assert_final_heap_state(); + res = pthread_join(thr, NULL); + assert(res == 0); return 0; } - diff --git a/test/test_browser.py b/test/test_browser.py index 09581f1cdf633..e045f95cde4db 100644 --- a/test/test_browser.py +++ b/test/test_browser.py @@ -4726,27 +4726,27 @@ def test_minimal_runtime_hello_thread(self, opts): # Tests memory growth in pthreads mode, but still on the main thread. @parameterized({ - '': ([],), - 'proxy': (['-sPROXY_TO_PTHREAD'],), + '': ([], 1), + 'proxy': (['-sPROXY_TO_PTHREAD'], 2), }) @no_2gb('uses INITIAL_MEMORY') @no_4gb('uses INITIAL_MEMORY') - def test_pthread_growth_mainthread(self, emcc_args): - self.emcc_args.remove('-Werror') - self.btest_exit('pthread/test_pthread_memory_growth_mainthread.c', emcc_args=['-pthread', '-sPTHREAD_POOL_SIZE=2', '-sALLOW_MEMORY_GROWTH', '-sINITIAL_MEMORY=32MB', '-sMAXIMUM_MEMORY=256MB'] + emcc_args) + def test_pthread_growth_mainthread(self, emcc_args, pthread_pool_size): + self.set_setting('PTHREAD_POOL_SIZE', pthread_pool_size) + self.btest_exit('pthread/test_pthread_memory_growth_mainthread.c', emcc_args=['-Wno-pthreads-mem-growth', '-pthread', '-sALLOW_MEMORY_GROWTH', '-sINITIAL_MEMORY=32MB', '-sMAXIMUM_MEMORY=256MB'] + emcc_args) # Tests memory growth in a pthread. @parameterized({ '': ([],), 'assert': (['-sASSERTIONS'],), - 'proxy': (['-sPROXY_TO_PTHREAD'],), + 'proxy': (['-sPROXY_TO_PTHREAD'], 2), 'minimal': (['-sMINIMAL_RUNTIME', '-sMODULARIZE', '-sEXPORT_NAME=MyModule'],), }) @no_2gb('uses INITIAL_MEMORY') @no_4gb('uses INITIAL_MEMORY') - def test_pthread_growth(self, emcc_args): - self.emcc_args.remove('-Werror') - self.btest_exit('pthread/test_pthread_memory_growth.c', emcc_args=['-pthread', '-sPTHREAD_POOL_SIZE=2', '-sALLOW_MEMORY_GROWTH', '-sINITIAL_MEMORY=32MB', '-sMAXIMUM_MEMORY=256MB', '-g'] + emcc_args) + def test_pthread_growth(self, emcc_args, pthread_pool_size = 1): + self.set_setting('PTHREAD_POOL_SIZE', pthread_pool_size) + self.btest_exit('pthread/test_pthread_memory_growth.c', emcc_args=['-Wno-pthreads-mem-growth', '-pthread', '-sALLOW_MEMORY_GROWTH', '-sINITIAL_MEMORY=32MB', '-sMAXIMUM_MEMORY=256MB'] + emcc_args) # Tests that time in a pthread is relative to the main thread, so measurements # on different threads are still monotonic, as if checking a single central From 20e10ecf9bc9566b29b09ccbc1c9a88774a1fd2a Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 14 May 2025 16:22:21 +0100 Subject: [PATCH 08/14] Fix GROWABLE_HEAP with new rigorous tests --- src/growableHeap.js | 6 ++--- test/js_optimizer/growableHeap-output.js | 12 ++++----- ...codesize_minimal_pthreads_memgrowth.gzsize | 2 +- ...codesize_minimal_pthreads_memgrowth.jssize | 2 +- test/pthread/test_pthread_memory_growth.c | 15 ++++++----- .../test_pthread_memory_growth_mainthread.c | 25 ++++++++++--------- test/test_other.py | 4 +-- tools/acorn-optimizer.mjs | 23 ++++++++++++++--- 8 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/growableHeap.js b/src/growableHeap.js index abfe289bbd0be..88650f1a4307a 100644 --- a/src/growableHeap.js +++ b/src/growableHeap.js @@ -6,9 +6,9 @@ // Support for growable heap + pthreads, where the buffer may change, so JS views // must be updated. -function GROWABLE_HEAP(arr) { - if (wasmMemory.buffer != arr.buffer) { +function GROWABLE_HEAP() { + // `updateMemoryViews` updates all the views simultaneously, so it's enough to check any of them. + if (wasmMemory.buffer != HEAP8.buffer) { updateMemoryViews(); } - return arr; } diff --git a/test/js_optimizer/growableHeap-output.js b/test/js_optimizer/growableHeap-output.js index 8a7f1b4a68ae8..61c6070edad13 100644 --- a/test/js_optimizer/growableHeap-output.js +++ b/test/js_optimizer/growableHeap-output.js @@ -1,13 +1,13 @@ function f() { var $2 = 0; - GROWABLE_HEAP(HEAP32)[$0 >> 2] = $2 + 1; - $9 = GROWABLE_HEAP(HEAPU8)[$2 >> 0] | 0; - +GROWABLE_HEAP(HEAPF64)[x >> 3]; - GROWABLE_HEAP(HEAP64)[x >> 3] = GROWABLE_HEAP(HEAP64)[y >> 3]; + (GROWABLE_HEAP(), HEAP32)[$0 >> 2] = $2 + 1; + $9 = (GROWABLE_HEAP(), HEAPU8)[$2 >> 0] | 0; + +(GROWABLE_HEAP(), HEAPF64)[x >> 3]; + (GROWABLE_HEAP(), HEAP64)[x >> 3] = (GROWABLE_HEAP(), HEAP64)[y >> 3]; } function libraryFunc(ptr, val) { - if (ptr < GROWABLE_HEAP(HEAP8).length) { - Atomics.wait(GROWABLE_HEAP(HEAP32), ptr, val); + if (ptr < (GROWABLE_HEAP(), HEAP8).length) { + Atomics.wait((GROWABLE_HEAP(), HEAP32), ptr, val); } } diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize index 8597d55482f06..5255d8eda08aa 100644 --- a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize @@ -1 +1 @@ -4181 +4184 diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize index 5df4b65f6340c..b86d17255235e 100644 --- a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize @@ -1 +1 @@ -8654 +8695 diff --git a/test/pthread/test_pthread_memory_growth.c b/test/pthread/test_pthread_memory_growth.c index 2b8d9486d868d..8f80b67223cb5 100644 --- a/test/pthread/test_pthread_memory_growth.c +++ b/test/pthread/test_pthread_memory_growth.c @@ -7,6 +7,7 @@ #include #include #include +#include // We want to test that JavaScript access in the main thread automatically updates the memory views if the heap has grown from the pthread meanwhile. // @@ -18,12 +19,13 @@ // etc., not even `puts`/`printf` for logging - as all of those access the heap from JS on their own and might update the memory views too early. // Not using standard proxying mechanisms also means we need to drop down all the way to raw atomics. -EM_JS(void, assert_initial_heap_state, (), { - console.log(`Checking initial heap state on the ${ENVIRONMENT_IS_PTHREAD ? 'worker' : 'main'} thread`); +EM_JS(void, assert_initial_heap_state, (bool isWorker), { + console.log(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); }); -EM_JS(void, assert_final_heap_state, (const char* buffer, int finalHeapSize), { +EM_JS(void, assert_final_heap_state, (bool isWorker, const char* buffer, int finalHeapSize), { + console.log(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length > finalHeapSize, "end with >64MB"); assert(HEAP8[buffer] === 42, "readable from JS"); }); @@ -32,7 +34,7 @@ EM_JS(void, assert_final_heap_state, (const char* buffer, int finalHeapSize), { static void *thread_start(void *arg) { - assert_initial_heap_state(); + assert_initial_heap_state(true); // allocate more memory than we currently have, forcing a growth char* buffer = malloc(FINAL_HEAP_SIZE); assert(buffer); @@ -40,12 +42,13 @@ static void *thread_start(void *arg) buffer += FINAL_HEAP_SIZE - 1; *buffer = 42; *(const char *_Atomic *)arg = buffer; + assert_final_heap_state(true, buffer, FINAL_HEAP_SIZE); return NULL; } int main() { - assert_initial_heap_state(); + assert_initial_heap_state(false); pthread_t thr; const char *_Atomic buffer = NULL; @@ -55,7 +58,7 @@ int main() while (!buffer); assert(*buffer == 42); // should see the value the thread wrote - assert_final_heap_state(buffer, FINAL_HEAP_SIZE); + assert_final_heap_state(false, buffer, FINAL_HEAP_SIZE); res = pthread_join(thr, NULL); assert(res == 0); diff --git a/test/pthread/test_pthread_memory_growth_mainthread.c b/test/pthread/test_pthread_memory_growth_mainthread.c index 0bc1dd2645b1d..20b8c0adbda72 100644 --- a/test/pthread/test_pthread_memory_growth_mainthread.c +++ b/test/pthread/test_pthread_memory_growth_mainthread.c @@ -7,6 +7,7 @@ #include #include #include +#include // We want to test that JavaScript access in a pthread automatically updates the memory views if the heap has grown on the main thread meanwhile. // @@ -18,13 +19,13 @@ // etc., not even `puts`/`printf` for logging - as all of those access the heap from JS on their own and might update the memory views too early. // Not using standard proxying mechanisms also means we need to drop down all the way to raw atomics. -EM_JS(void, js_assert_initial_heap_state, (), { - console.log(`Checking initial heap state on the ${ENVIRONMENT_IS_PTHREAD ? 'worker' : 'main'} thread`); +EM_JS(void, js_assert_initial_heap_state, (bool isWorker), { + console.log(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); }); -EM_JS(void, js_assert_final_heap_state, (const char* buffer, int finalHeapSize), { - console.log(`Checking final heap state on the ${ENVIRONMENT_IS_PTHREAD ? 'worker' : 'main'} thread`); +EM_JS(void, js_assert_final_heap_state, (bool isWorker, const char* buffer, int finalHeapSize), { + console.log(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length > finalHeapSize, "end with >64MB"); assert(HEAP8[buffer] === 42, "readable from JS"); }); @@ -37,32 +38,32 @@ _Atomic enum state { const char *_Atomic buffer; -static void assert_initial_heap_state() +static void assert_initial_heap_state(bool is_worker) { assert(state == INITIAL_STATE); assert(buffer == NULL); - js_assert_initial_heap_state(); + js_assert_initial_heap_state(is_worker); } #define FINAL_HEAP_SIZE (64 * 1024 * 1024) -static void assert_final_heap_state() +static void assert_final_heap_state(bool is_worker) { assert(state == FINAL_STATE); assert(buffer != NULL); assert(*buffer == 42); - js_assert_final_heap_state(buffer, FINAL_HEAP_SIZE); + js_assert_final_heap_state(is_worker, buffer, FINAL_HEAP_SIZE); } static void *thread_start(void *arg) { - assert_initial_heap_state(); + assert_initial_heap_state(true); // Tell main thread that we checked the initial state and it can allocate. state = THREAD_CHECKED_INITIAL_STATE; // Wait for the main thread to allocate memory. while (state != FINAL_STATE); // Check the final heap state. - assert_final_heap_state(); + assert_final_heap_state(true); return NULL; } @@ -82,14 +83,14 @@ int main() int res = pthread_create(&thr, NULL, thread_start, NULL); assert(res == 0); // Check initial state in both threads before allocating more memory. - assert_initial_heap_state(); + assert_initial_heap_state(false); while (state != THREAD_CHECKED_INITIAL_STATE); // allocate more memory than we currently have, forcing a growth buffer = alloc_beyond_initial_heap(); state = FINAL_STATE; - assert_final_heap_state(); + assert_final_heap_state(false); res = pthread_join(thr, NULL); assert(res == 0); diff --git a/test/test_other.py b/test/test_other.py index 4d4cd390abd26..daef12ece648b 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -10544,9 +10544,9 @@ def test_pthreads_growth_and_unsigned(self): '-sMAXIMUM_MEMORY=4GB', '-sALLOW_MEMORY_GROWTH']) # growable-heap must not interfere with heap unsigning, and vice versa: # we must have both applied, that is - # - GROWABLE_HEAP() wraps HEAP8 + # - GROWABLE_HEAP() runs before HEAP8 # - $0 gets an >>> 0 unsigning - self.assertContained('GROWABLE_HEAP(HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', + self.assertContained('(GROWABLE_HEAP(), HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', read_file('a.out.js')) @parameterized({ diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index f1b80b560bce8..99f60fe0a16c7 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1305,11 +1305,10 @@ function growableHeap(ast) { } }, AssignmentExpression: (node) => { - if (node.left.type === 'Identifier' && isEmscriptenHEAP(node.left.name)) { + if (node.left.type !== 'Identifier') { // Don't transform initial setup of the arrays. - return; - } growableHeap(node.left); + } growableHeap(node.right); }, VariableDeclaration: (node) => { @@ -1324,7 +1323,23 @@ function growableHeap(ast) { }, Identifier: (node) => { if (isEmscriptenHEAP(node.name)) { - makeCallExpression(node, 'GROWABLE_HEAP', [{...node}]); + // Transform `HEAPxx` into `(GROWABLE_HEAP(), HEAPxx)`. + // Important: don't just do `GROWABLE_HEAP(HEAPxx)` because `GROWABLE_HEAP` reassigns `HEAPxx` + // and we want to get an updated value after that reassignment. + Object.assign(node, { + type: 'SequenceExpression', + expressions: [ + createNode({ + type: 'CallExpression', + callee: createNode({ + type: 'Identifier', + name: 'GROWABLE_HEAP', + }), + arguments: [], + }), + {...node}, + ], + }); } }, }); From 7f623841c8856da30599c6200b3b222389883d34 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 14 May 2025 16:25:12 +0100 Subject: [PATCH 09/14] Format --- tools/acorn-optimizer.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 99f60fe0a16c7..2ea9db03e0f88 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1307,7 +1307,7 @@ function growableHeap(ast) { AssignmentExpression: (node) => { if (node.left.type !== 'Identifier') { // Don't transform initial setup of the arrays. - growableHeap(node.left); + growableHeap(node.left); } growableHeap(node.right); }, From cc25a48b0bf353c40ba49d30d5b06451bef58840 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 14 May 2025 17:18:15 +0100 Subject: [PATCH 10/14] Rename helper --- site/source/docs/porting/pthreads.rst | 2 +- src/growableHeap.js | 2 +- test/js_optimizer/growableHeap-output.js | 12 ++++++------ test/test_other.py | 2 +- tools/acorn-optimizer.mjs | 12 ++++++------ 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/site/source/docs/porting/pthreads.rst b/site/source/docs/porting/pthreads.rst index a8507a19b8927..0c46422d21295 100644 --- a/site/source/docs/porting/pthreads.rst +++ b/site/source/docs/porting/pthreads.rst @@ -144,7 +144,7 @@ The Emscripten implementation for the pthreads API should follow the POSIX stand - Note that the function emscripten_num_logical_cores() will always return the value of navigator.hardwareConcurrency, i.e. the number of logical cores on the system, even when shared memory is not supported. This means that it is possible for emscripten_num_logical_cores() to return a value greater than 1, while at the same time emscripten_has_threading_support() can return false. The return value of emscripten_has_threading_support() denotes whether the browser has shared memory support available. -- Pthreads + memory growth (``ALLOW_MEMORY_GROWTH``) is especially tricky, see `Wasm design issue #1271 `_. This currently causes JS accessing the Wasm memory to be slow - but this will likely only be noticeable if the JS does large amounts of memory reads and writes (Wasm runs at full speed, so moving work over can fix this). This also requires that your JS be aware that the HEAP* views may need to be updated - JS code embedded with ``--js-library`` etc will automatically be transformed to use the ``GROWABLE_HEAP`` helper function, but external code that uses ``Module.HEAP*`` directly may encounter problems with views being smaller than memory. +- Pthreads + memory growth (``ALLOW_MEMORY_GROWTH``) is especially tricky, see `Wasm design issue #1271 `_. This currently causes JS accessing the Wasm memory to be slow - but this will likely only be noticeable if the JS does large amounts of memory reads and writes (Wasm runs at full speed, so moving work over can fix this). This also requires that your JS be aware that the HEAP* views may need to be updated - JS code embedded with ``--js-library`` etc will automatically be transformed to auto-update memory views before each access, but external code that uses ``Module.HEAP*`` directly may encounter problems with views being smaller than memory. .. _Allocator_performance: diff --git a/src/growableHeap.js b/src/growableHeap.js index 88650f1a4307a..d560674202b46 100644 --- a/src/growableHeap.js +++ b/src/growableHeap.js @@ -6,7 +6,7 @@ // Support for growable heap + pthreads, where the buffer may change, so JS views // must be updated. -function GROWABLE_HEAP() { +function maybeUpdateMemoryViews() { // `updateMemoryViews` updates all the views simultaneously, so it's enough to check any of them. if (wasmMemory.buffer != HEAP8.buffer) { updateMemoryViews(); diff --git a/test/js_optimizer/growableHeap-output.js b/test/js_optimizer/growableHeap-output.js index 61c6070edad13..244d9ad5c7cea 100644 --- a/test/js_optimizer/growableHeap-output.js +++ b/test/js_optimizer/growableHeap-output.js @@ -1,13 +1,13 @@ function f() { var $2 = 0; - (GROWABLE_HEAP(), HEAP32)[$0 >> 2] = $2 + 1; - $9 = (GROWABLE_HEAP(), HEAPU8)[$2 >> 0] | 0; - +(GROWABLE_HEAP(), HEAPF64)[x >> 3]; - (GROWABLE_HEAP(), HEAP64)[x >> 3] = (GROWABLE_HEAP(), HEAP64)[y >> 3]; + (maybeUpdateMemoryViews(), HEAP32)[$0 >> 2] = $2 + 1; + $9 = (maybeUpdateMemoryViews(), HEAPU8)[$2 >> 0] | 0; + +(maybeUpdateMemoryViews(), HEAPF64)[x >> 3]; + (maybeUpdateMemoryViews(), HEAP64)[x >> 3] = (maybeUpdateMemoryViews(), HEAP64)[y >> 3]; } function libraryFunc(ptr, val) { - if (ptr < (GROWABLE_HEAP(), HEAP8).length) { - Atomics.wait((GROWABLE_HEAP(), HEAP32), ptr, val); + if (ptr < (maybeUpdateMemoryViews(), HEAP8).length) { + Atomics.wait((maybeUpdateMemoryViews(), HEAP32), ptr, val); } } diff --git a/test/test_other.py b/test/test_other.py index daef12ece648b..4390c9fdaf11e 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -10546,7 +10546,7 @@ def test_pthreads_growth_and_unsigned(self): # we must have both applied, that is # - GROWABLE_HEAP() runs before HEAP8 # - $0 gets an >>> 0 unsigning - self.assertContained('(GROWABLE_HEAP(), HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', + self.assertContained('(maybeUpdateMemoryViews(), HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', read_file('a.out.js')) @parameterized({ diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 2ea9db03e0f88..24474bf388df2 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1288,17 +1288,17 @@ function littleEndianHeap(ast) { }); } -// Instrument heap accesses to call GROWABLE_HEAP helper function, which allows +// Instrument heap accesses to call maybeUpdateMemoryViews helper function, which allows // pthreads + memory growth to work (we check if the memory was grown on another thread // in each access), see #8365. function growableHeap(ast) { recursiveWalk(ast, { FunctionDeclaration(node, c) { - // Do not recurse into to GROWABLE_HEAP helper function itself. + // Do not recurse into the helper function itself. if ( !( node.id.type === 'Identifier' && - (node.id.name === 'GROWABLE_HEAP' || node.id.name === 'LE_HEAP_UPDATE') + (node.id.name === 'maybeUpdateMemoryViews' || node.id.name === 'LE_HEAP_UPDATE') ) ) { c(node.body); @@ -1323,8 +1323,8 @@ function growableHeap(ast) { }, Identifier: (node) => { if (isEmscriptenHEAP(node.name)) { - // Transform `HEAPxx` into `(GROWABLE_HEAP(), HEAPxx)`. - // Important: don't just do `GROWABLE_HEAP(HEAPxx)` because `GROWABLE_HEAP` reassigns `HEAPxx` + // Transform `HEAPxx` into `(maybeUpdateMemoryViews(), HEAPxx)`. + // Important: don't just do `maybeUpdateMemoryViews(HEAPxx)` because `maybeUpdateMemoryViews` reassigns `HEAPxx` // and we want to get an updated value after that reassignment. Object.assign(node, { type: 'SequenceExpression', @@ -1333,7 +1333,7 @@ function growableHeap(ast) { type: 'CallExpression', callee: createNode({ type: 'Identifier', - name: 'GROWABLE_HEAP', + name: 'maybeUpdateMemoryViews', }), arguments: [], }), From 625e32085d68bb4f42621c18bf5da1834beb8159 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Wed, 14 May 2025 17:46:23 +0100 Subject: [PATCH 11/14] Rename again --- src/growableHeap.js | 2 +- test/js_optimizer/growableHeap-output.js | 12 ++++++------ test/test_other.py | 2 +- tools/acorn-optimizer.mjs | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/growableHeap.js b/src/growableHeap.js index d560674202b46..5eca81d8840b5 100644 --- a/src/growableHeap.js +++ b/src/growableHeap.js @@ -6,7 +6,7 @@ // Support for growable heap + pthreads, where the buffer may change, so JS views // must be updated. -function maybeUpdateMemoryViews() { +function growMemViews() { // `updateMemoryViews` updates all the views simultaneously, so it's enough to check any of them. if (wasmMemory.buffer != HEAP8.buffer) { updateMemoryViews(); diff --git a/test/js_optimizer/growableHeap-output.js b/test/js_optimizer/growableHeap-output.js index 244d9ad5c7cea..4c273855e1438 100644 --- a/test/js_optimizer/growableHeap-output.js +++ b/test/js_optimizer/growableHeap-output.js @@ -1,13 +1,13 @@ function f() { var $2 = 0; - (maybeUpdateMemoryViews(), HEAP32)[$0 >> 2] = $2 + 1; - $9 = (maybeUpdateMemoryViews(), HEAPU8)[$2 >> 0] | 0; - +(maybeUpdateMemoryViews(), HEAPF64)[x >> 3]; - (maybeUpdateMemoryViews(), HEAP64)[x >> 3] = (maybeUpdateMemoryViews(), HEAP64)[y >> 3]; + (growMemViews(), HEAP32)[$0 >> 2] = $2 + 1; + $9 = (growMemViews(), HEAPU8)[$2 >> 0] | 0; + +(growMemViews(), HEAPF64)[x >> 3]; + (growMemViews(), HEAP64)[x >> 3] = (growMemViews(), HEAP64)[y >> 3]; } function libraryFunc(ptr, val) { - if (ptr < (maybeUpdateMemoryViews(), HEAP8).length) { - Atomics.wait((maybeUpdateMemoryViews(), HEAP32), ptr, val); + if (ptr < (growMemViews(), HEAP8).length) { + Atomics.wait((growMemViews(), HEAP32), ptr, val); } } diff --git a/test/test_other.py b/test/test_other.py index 4390c9fdaf11e..c1f67b0331ed4 100644 --- a/test/test_other.py +++ b/test/test_other.py @@ -10546,7 +10546,7 @@ def test_pthreads_growth_and_unsigned(self): # we must have both applied, that is # - GROWABLE_HEAP() runs before HEAP8 # - $0 gets an >>> 0 unsigning - self.assertContained('(maybeUpdateMemoryViews(), HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', + self.assertContained('(growMemViews(), HEAP8).set([ 1, 2, 3 ], $0 >>> 0)', read_file('a.out.js')) @parameterized({ diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index 24474bf388df2..f02fdea393e7c 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1288,7 +1288,7 @@ function littleEndianHeap(ast) { }); } -// Instrument heap accesses to call maybeUpdateMemoryViews helper function, which allows +// Instrument heap accesses to call growMemViews helper function, which allows // pthreads + memory growth to work (we check if the memory was grown on another thread // in each access), see #8365. function growableHeap(ast) { @@ -1298,7 +1298,7 @@ function growableHeap(ast) { if ( !( node.id.type === 'Identifier' && - (node.id.name === 'maybeUpdateMemoryViews' || node.id.name === 'LE_HEAP_UPDATE') + (node.id.name === 'growMemViews' || node.id.name === 'LE_HEAP_UPDATE') ) ) { c(node.body); @@ -1323,8 +1323,8 @@ function growableHeap(ast) { }, Identifier: (node) => { if (isEmscriptenHEAP(node.name)) { - // Transform `HEAPxx` into `(maybeUpdateMemoryViews(), HEAPxx)`. - // Important: don't just do `maybeUpdateMemoryViews(HEAPxx)` because `maybeUpdateMemoryViews` reassigns `HEAPxx` + // Transform `HEAPxx` into `(growMemViews(), HEAPxx)`. + // Important: don't just do `growMemViews(HEAPxx)` because `growMemViews` reassigns `HEAPxx` // and we want to get an updated value after that reassignment. Object.assign(node, { type: 'SequenceExpression', @@ -1333,7 +1333,7 @@ function growableHeap(ast) { type: 'CallExpression', callee: createNode({ type: 'Identifier', - name: 'maybeUpdateMemoryViews', + name: 'growMemViews', }), arguments: [], }), From aad946d3940508b431fcf8387ac959c7cb5adee9 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Thu, 15 May 2025 22:07:32 +0100 Subject: [PATCH 12/14] Address review comments --- test/pthread/test_pthread_memory_growth.c | 53 +++++++++++++------ .../test_pthread_memory_growth_mainthread.c | 20 ++++--- 2 files changed, 49 insertions(+), 24 deletions(-) diff --git a/test/pthread/test_pthread_memory_growth.c b/test/pthread/test_pthread_memory_growth.c index 8f80b67223cb5..d2219b3021b61 100644 --- a/test/pthread/test_pthread_memory_growth.c +++ b/test/pthread/test_pthread_memory_growth.c @@ -9,22 +9,28 @@ #include #include -// We want to test that JavaScript access in the main thread automatically updates the memory views if the heap has grown from the pthread meanwhile. +// We want to test that JavaScript access in the main thread automatically +// updates the memory views if the heap has grown from the pthread meanwhile. // -// Checking this correctly is somewhat tricky because a lot of Emscripten APIs access heap on their own, so the memory views might be updated -// by them before we explicitly check the state in our own JS routines below, which leads to false positives (test passing even though in -// isolation the heap access in our own JS is not the one updating the memory views). +// Checking this correctly is somewhat tricky because a lot of Emscripten APIs +// access heap on their own, so the memory views might be updated by them before +// we explicitly check the state in our own JS routines below, which leads to +// false positives (test passing even though in isolation the heap access in our +// own JS is not the one updating the memory views). // -// To test it in isolation, we need to use only EM_JS - which is as close to a pure JS call as we can get - and not EM_ASM, pthread_join, proxying -// etc., not even `puts`/`printf` for logging - as all of those access the heap from JS on their own and might update the memory views too early. -// Not using standard proxying mechanisms also means we need to drop down all the way to raw atomics. +// To test it in isolation, we need to use only EM_JS - which is as close to a +// pure JS call as we can get - and not EM_ASM, pthread_join, proxying etc., not +// even `puts`/`printf` for logging - as all of those access the heap from JS on +// their own and might update the memory views too early. Not using standard +// proxying mechanisms also means we need to drop down all the way to raw +// atomics. EM_JS(void, assert_initial_heap_state, (bool isWorker), { console.log(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); }); -EM_JS(void, assert_final_heap_state, (bool isWorker, const char* buffer, int finalHeapSize), { +EM_JS(void, js_assert_final_heap_state, (bool isWorker, const char* buffer, int finalHeapSize), { console.log(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length > finalHeapSize, "end with >64MB"); assert(HEAP8[buffer] === 42, "readable from JS"); @@ -32,17 +38,31 @@ EM_JS(void, assert_final_heap_state, (bool isWorker, const char* buffer, int fin #define FINAL_HEAP_SIZE (64 * 1024 * 1024) -static void *thread_start(void *arg) +static char *alloc_beyond_initial_heap() { - assert_initial_heap_state(true); - // allocate more memory than we currently have, forcing a growth - char* buffer = malloc(FINAL_HEAP_SIZE); + char *buffer = malloc(FINAL_HEAP_SIZE); assert(buffer); // Write value at the end of the buffer to check that any thread can access addresses beyond the initial heap size. buffer += FINAL_HEAP_SIZE - 1; *buffer = 42; - *(const char *_Atomic *)arg = buffer; - assert_final_heap_state(true, buffer, FINAL_HEAP_SIZE); + return buffer; +} + +const char *_Atomic buffer = NULL; + +static void assert_final_heap_state(bool is_worker) +{ + assert(buffer != NULL); + assert(*buffer == 42); + js_assert_final_heap_state(is_worker, buffer, FINAL_HEAP_SIZE); +} + +static void *thread_start(void *arg) +{ + assert_initial_heap_state(true); + // allocate more memory than we currently have, forcing a growth + buffer = alloc_beyond_initial_heap(); + assert_final_heap_state(true); return NULL; } @@ -51,14 +71,13 @@ int main() assert_initial_heap_state(false); pthread_t thr; - const char *_Atomic buffer = NULL; - int res = pthread_create(&thr, NULL, thread_start, &buffer); + int res = pthread_create(&thr, NULL, thread_start, NULL); assert(res == 0); while (!buffer); assert(*buffer == 42); // should see the value the thread wrote - assert_final_heap_state(false, buffer, FINAL_HEAP_SIZE); + assert_final_heap_state(false); res = pthread_join(thr, NULL); assert(res == 0); diff --git a/test/pthread/test_pthread_memory_growth_mainthread.c b/test/pthread/test_pthread_memory_growth_mainthread.c index 20b8c0adbda72..c33d4c55ef6f2 100644 --- a/test/pthread/test_pthread_memory_growth_mainthread.c +++ b/test/pthread/test_pthread_memory_growth_mainthread.c @@ -9,15 +9,21 @@ #include #include -// We want to test that JavaScript access in a pthread automatically updates the memory views if the heap has grown on the main thread meanwhile. +// We want to test that JavaScript access in a pthread automatically updates the +// memory views if the heap has grown on the main thread meanwhile. // -// Checking this correctly is somewhat tricky because a lot of Emscripten APIs access heap on their own, so the memory views might be updated -// by them before we explicitly check the state in our own JS routines below, which leads to false positives (test passing even though in -// isolation the heap access in our own JS is not the one updating the memory views). +// Checking this correctly is somewhat tricky because a lot of Emscripten APIs +// access heap on their own, so the memory views might be updated by them before +// we explicitly check the state in our own JS routines below, which leads to +// false positives (test passing even though in isolation the heap access in our +// own JS is not the one updating the memory views). // -// To test it in isolation, we need to use only EM_JS - which is as close to a pure JS call as we can get - and not EM_ASM, pthread_join, proxying -// etc., not even `puts`/`printf` for logging - as all of those access the heap from JS on their own and might update the memory views too early. -// Not using standard proxying mechanisms also means we need to drop down all the way to raw atomics. +// To test it in isolation, we need to use only EM_JS - which is as close to a +// pure JS call as we can get - and not EM_ASM, pthread_join, proxying etc., not +// even `puts`/`printf` for logging - as all of those access the heap from JS on +// their own and might update the memory views too early. Not using standard +// proxying mechanisms also means we need to drop down all the way to raw +// atomics. EM_JS(void, js_assert_initial_heap_state, (bool isWorker), { console.log(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); From bdee9d8f82875a82eda3c07efe216bdb76e1eb11 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 16 May 2025 01:05:09 +0100 Subject: [PATCH 13/14] Address few more review concerns --- test/pthread/test_pthread_memory_growth.c | 4 ++-- test/pthread/test_pthread_memory_growth_mainthread.c | 4 ++-- tools/acorn-optimizer.mjs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/pthread/test_pthread_memory_growth.c b/test/pthread/test_pthread_memory_growth.c index d2219b3021b61..ef4f4a7e3e220 100644 --- a/test/pthread/test_pthread_memory_growth.c +++ b/test/pthread/test_pthread_memory_growth.c @@ -26,12 +26,12 @@ // atomics. EM_JS(void, assert_initial_heap_state, (bool isWorker), { - console.log(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); + dbg(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); }); EM_JS(void, js_assert_final_heap_state, (bool isWorker, const char* buffer, int finalHeapSize), { - console.log(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); + dbg(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length > finalHeapSize, "end with >64MB"); assert(HEAP8[buffer] === 42, "readable from JS"); }); diff --git a/test/pthread/test_pthread_memory_growth_mainthread.c b/test/pthread/test_pthread_memory_growth_mainthread.c index c33d4c55ef6f2..cf6da94781b1d 100644 --- a/test/pthread/test_pthread_memory_growth_mainthread.c +++ b/test/pthread/test_pthread_memory_growth_mainthread.c @@ -26,12 +26,12 @@ // atomics. EM_JS(void, js_assert_initial_heap_state, (bool isWorker), { - console.log(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); + dbg(`Checking initial heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB"); }); EM_JS(void, js_assert_final_heap_state, (bool isWorker, const char* buffer, int finalHeapSize), { - console.log(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); + dbg(`Checking final heap state on the ${isWorker ? 'worker' : 'main'} thread`); assert(HEAP8.length > finalHeapSize, "end with >64MB"); assert(HEAP8[buffer] === 42, "readable from JS"); }); diff --git a/tools/acorn-optimizer.mjs b/tools/acorn-optimizer.mjs index f02fdea393e7c..9f2d861c0d289 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1306,7 +1306,7 @@ function growableHeap(ast) { }, AssignmentExpression: (node) => { if (node.left.type !== 'Identifier') { - // Don't transform initial setup of the arrays. + // Don't transform `HEAPxx =` assignments. growableHeap(node.left); } growableHeap(node.right); From a9354cf396f9f9e58f12882d0ac6fb0efc38b023 Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Fri, 16 May 2025 21:34:13 +0100 Subject: [PATCH 14/14] Rebaseline --- .../codesize/test_codesize_minimal_pthreads_memgrowth.gzsize | 2 +- .../codesize/test_codesize_minimal_pthreads_memgrowth.jssize | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize index 5255d8eda08aa..f3f9da2b2a6b1 100644 --- a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize @@ -1 +1 @@ -4184 +3991 diff --git a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize index b86d17255235e..a617ecc67e53f 100644 --- a/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize @@ -1 +1 @@ -8695 +8263