diff --git a/site/source/docs/porting/pthreads.rst b/site/source/docs/porting/pthreads.rst index 6f00b3971d6c3..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 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 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 099df836d0768..5eca81d8840b5 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() { +function growMemViews() { + // `updateMemoryViews` updates all the views simultaneously, so it's enough to check any of them. if (wasmMemory.buffer != HEAP8.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; } 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; diff --git a/src/modules.mjs b/src/modules.mjs index ef59b326d50d2..24eb9dd2d63e3 100644 --- a/src/modules.mjs +++ b/src/modules.mjs @@ -455,20 +455,6 @@ function exportRuntimeSymbols() { runtimeElements.push('HEAP_DATA_VIEW'); } - 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', - ); - } if (USE_OFFSET_CONVERTER) { runtimeElements.push('WasmOffsetConverter'); } diff --git a/test/js_optimizer/growableHeap-output.js b/test/js_optimizer/growableHeap-output.js index 0e70a486b170c..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; - 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]; + (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 < GROWABLE_HEAP_I8().length) { - Atomics.wait(GROWABLE_HEAP_I32(), ptr, val); + if (ptr < (growMemViews(), HEAP8).length) { + Atomics.wait((growMemViews(), HEAP32), ptr, val); } } 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..f3f9da2b2a6b1 --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.gzsize @@ -0,0 +1 @@ +3991 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..a617ecc67e53f --- /dev/null +++ b/test/other/codesize/test_codesize_minimal_pthreads_memgrowth.jssize @@ -0,0 +1 @@ +8263 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/pthread/test_pthread_memory_growth.c b/test/pthread/test_pthread_memory_growth.c index ce9e370d214d5..ef4f4a7e3e220 100644 --- a/test/pthread/test_pthread_memory_growth.c +++ b/test/pthread/test_pthread_memory_growth.c @@ -4,42 +4,83 @@ // found in the LICENSE file. #include -#include -#include #include #include -#include +#include +#include -static void *thread_start(void *arg) +// 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, (bool isWorker), { + 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), { + 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"); +}); + +#define FINAL_HEAP_SIZE (64 * 1024 * 1024) + +static char *alloc_beyond_initial_heap() { - // 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); + 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; } int main() { - printf("prep\n"); + assert_initial_heap_state(false); pthread_t thr; + int res = pthread_create(&thr, NULL, thread_start, NULL); + 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(false); + + 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..cf6da94781b1d 100644 --- a/test/pthread/test_pthread_memory_growth_mainthread.c +++ b/test/pthread/test_pthread_memory_growth_mainthread.c @@ -4,51 +4,101 @@ // found in the LICENSE file. #include -#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, (bool isWorker), { + 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), { + 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"); +}); + +_Atomic enum state { + INITIAL_STATE, + THREAD_CHECKED_INITIAL_STATE, + FINAL_STATE +} state; + +const char *_Atomic buffer; + +static void assert_initial_heap_state(bool is_worker) { - char* buffer = (char*)arg; - assert(buffer); + assert(state == INITIAL_STATE); + assert(buffer == NULL); + js_assert_initial_heap_state(is_worker); +} + +#define FINAL_HEAP_SIZE (64 * 1024 * 1024) + +static void assert_final_heap_state(bool is_worker) +{ + 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(is_worker, 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(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(true); + 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(false); + 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(false); + 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 diff --git a/test/test_other.py b/test/test_other.py index c196254cf1422..c1f67b0331ed4 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 @@ -10540,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() runs before HEAP8 # - $0 gets an >>> 0 unsigning - self.assertContained('GROWABLE_HEAP_I8().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 4a47979e3feed..9f2d861c0d289 100755 --- a/tools/acorn-optimizer.mjs +++ b/tools/acorn-optimizer.mjs @@ -1288,28 +1288,27 @@ function littleEndianHeap(ast) { }); } -// Instrument heap accesses to call GROWABLE_HEAP_* helper functions instead, 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) { recursiveWalk(ast, { FunctionDeclaration(node, c) { - // Do not recurse into to GROWABLE_HEAP_ helper functions themselves. + // Do not recurse into the helper function itself. if ( !( node.id.type === 'Identifier' && - (node.id.name.startsWith('GROWABLE_HEAP_') || node.id.name === 'LE_HEAP_UPDATE') + (node.id.name === 'growMemViews' || node.id.name === 'LE_HEAP_UPDATE') ) ) { c(node.body); } }, AssignmentExpression: (node) => { - if (node.left.type === 'Identifier' && isEmscriptenHEAP(node.left.name)) { - // Don't transform initial setup of the arrays. - return; + if (node.left.type !== 'Identifier') { + // Don't transform `HEAPxx =` assignments. + growableHeap(node.left); } - growableHeap(node.left); growableHeap(node.right); }, VariableDeclaration: (node) => { @@ -1323,50 +1322,24 @@ 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)) { + // 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', + expressions: [ + createNode({ + type: 'CallExpression', + callee: createNode({ + type: 'Identifier', + name: 'growMemViews', + }), + arguments: [], + }), + {...node}, + ], + }); } }, });