Working (but slow) support for pthreads + memory growth#8365
Conversation
…ere is a source of nondeterminism in the JS backend
|
Ah, I see some errors on CI that I missed in the subset of tests I ran locally. I don't see a way to turn this into a draft PR - is there? Anyhow, this is still a wip. |
| pthread_t thr; | ||
|
|
||
| printf("start\n"); | ||
| EM_ASM({ assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB") }); |
There was a problem hiding this comment.
Hmm, it's just test code, and convenient to do it all on a single line? Maybe I don't see the benefit to using EM_JS here?
There was a problem hiding this comment.
There's no benefit, other than everything should use EM_JS unless there's a good reason not to?
There was a problem hiding this comment.
In general yeah, but it's just not inline, so it's more typing and less immediately obvious to the reader - the only advantages of EM_ASM, basically. I agree EM_JS is the preferred thing for most normal code though.
|
Yeah these are in my custom JS library mix-ins, where I either malloc a buffer and write into it or create a typed array view for a C-created buffer to take data out. I'm not doing a lot of individual loads/stores; mostly data's coming as arguments on a callback, some of which are pointers/length pairs for buffers to export, which may be beyond the end of the old buffer views. I think it should work to run It might feel more natural to have view-creation accessors that wrap that check-and-update call, like:
|
|
Yeah, view creators are a natural fit too, added now. Also added docs about this. |
|
The view accessors look perfect, thanks! |
| #endif | ||
|
|
||
| #if MODULARIZE | ||
| // In modularize mode the wasmMemory and others are received in an onmessage, and that |
There was a problem hiding this comment.
should this be "in pthreads mode" rather than "modularize mode"
| pthread_t thr; | ||
|
|
||
| printf("start\n"); | ||
| EM_ASM({ assert(HEAP8.length === 32 * 1024 * 1024, "start at 32MB") }); |
There was a problem hiding this comment.
There's no benefit, other than everything should use EM_JS unless there's a good reason not to?
| def test_pthread_growth_mainthread(self): | ||
| def run(emcc_args=[]): | ||
| # Note that this test may not pass on Chrome until 75, due to https://bugs.chromium.org/p/v8/issues/detail?id=9062 | ||
| self.btest(path_from_root('tests', 'pthread', 'test_pthread_memory_growth_mainthread.c'), expected='1', args=['-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=2', '-s', 'ALLOW_MEMORY_GROWTH=1', '-s', 'TOTAL_MEMORY=32MB', '-s', 'WASM_MEM_MAX=256MB'] + emcc_args, also_asmjs=False) |
There was a problem hiding this comment.
should test in PROXY_TO_PTHREAD mode too?
There was a problem hiding this comment.
that won't work yet because of chromium issue 9065, same as test_pthread_growth. I'll add a PROXY_TO_PTHREAD mode in that test.
| }); | ||
| } | ||
|
|
||
| function growableHeap(ast) { |
There was a problem hiding this comment.
should this have some comment like "transform HEAP8 etc heap accesses to calls to GROWABLE_HEAP_LOAD_I8" or something?
…main thread can work (and hopefully soon unstable will make the thread version work too)
|
I tried to use chrome unstable so that at least part of the new tests here could pass, but it fails on unrelated things on circle, so that'll need to be looked at separately. As a result, for now the new tests are disabled. |
…ore#8365) (in wasm - no support in asm.js) Background: WebAssembly/design#1271 * Add GROWABLE_HEAP_* methods that replace each load and store in JS. These safely check if memory grew, and updates the buffer and views if so. The JS optimizer is used to instrument the JS in this way, so it works for user JS as well. * Show a warning about the slowness during compilation. * Make sbrk etc. threadsafe. * Rewrite brk using sbrk, to avoid writing two pieces of threadsafe code.
…ripten-core#8365)" This reverts commit a8164ac.
…ripten-core#8365)" This reverts commit a8164ac.
…ore#8365) (in wasm - no support in asm.js) Background: WebAssembly/design#1271 * Add GROWABLE_HEAP_* methods that replace each load and store in JS. These safely check if memory grew, and updates the buffer and views if so. The JS optimizer is used to instrument the JS in this way, so it works for user JS as well. * Show a warning about the slowness during compilation. * Make sbrk etc. threadsafe. * Rewrite brk using sbrk, to avoid writing two pieces of threadsafe code.
(in wasm - no support in asm.js)
Background: WebAssembly/design#1271
GROWABLE_HEAP_*methods that replace each load and store in JS. These safely check if memory grew, and updates the buffer and views if so. The JS optimizer is used to instrument the JS in this way, so it works for user JS as well.emscripten_resize_heapto the main thread, and handle races where a thread asks for growth but another thread grows it to a lower or higher value meanwhile - loop until successful, or an error is hit.Blocked on https://bugs.chromium.org/p/v8/issues/detail?id=9062 , so this is not fully tested yet - it doesn't test an actual memory growth, but it does test that instrumenting the code works.