Exiting pthreads postMessage to report the exit, which requires the main thread to receive events in order to reuse them

[omnisip]

Hi there,

I'm totally at a loss on what to expect for pthread behavior with my application. I'm using emscripten to implement a wasm module that acts as a portable SIMD library for performing operations. The library calls from Node.JS are designed to be synchronous and leverage memory passing. Inside the library, I'm spawning threads to expedite the operations (performing prefix sums and converting RGB image pixels to YUV). The threads wouldn't even start at the beginning before PTHREAD_POOL_SIZE. Now that I've added it, it works up until N threads have been consumed. PROXY_TO_PTHREAD doesn't seem to do anything meaningful at least not with respect to my application and Node.JS.

I figure if a pool is needed, why not just treat it like a pool instead of exhausting it? On the other hand, what are my alternatives if this can't get resolved? I need to be able to call rgb2y more than one time.

[kripken]

Do you mean that you create pthreads and stop using them - and they exit with pthread_exit normally - but they are not made available in the pool again? That sounds like a possible bug. I'd check if it happens in the browser, as that is more heavily tested - if it works there, it may be a bug specific to the node integration.

PROXY_TO_PTHREAD should also fix this, as it allows synchronous creation of pthreads. Coincidentally #12431 is adding docs and testing for that. If that doesn't work for you, it could be another bug.

If you don't have luck figuring this out, then attaching small testcases for these issues could allow us to investigate.

[omnisip]

That is exactly what happens. The pool of the threads will disappear in a 2 pass algorithm that spawns threads in each pass. Even though all the threads were used in the first pass, they exit and don't return to the pool for the second pass.

[sbc100]

Just checking, do you join with your used threads? Or set them as detached? I would imagine that have have to do one or the other before they can be recycled in the pool.

[omnisip]

std::async. I join on the futures and the tasks finish.

…

On Mon, Oct 5, 2020, 19:41 Sam Clegg ***@***.***> wrote: Just checking, do you join with your used threads? Or set them as detached? I would imagine that have have to do one or the other before they can be recycled in the pool. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#12400 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKQJUB2YYIJZBAYNYU3TTSJJYS5ANCNFSM4SA6V4JQ> .

[omnisip]

Do you still need test cases for this? Set your PTHREAD_POOL_SIZE to 16 and try something like this:

int foo (int p) {
return p;
}

void functionCalledFromNodeJs() {
   std::array<future, 16> futures;
 
// first pass
   for (int i = 0; i != 16; ++i) {
        futures[i] = std::async(std::launch::async, foo, i);
   }
   for (int i=0; i!=16; ++i) {
        futures[i].wait();
   }
    
// second pass
   for (int i = 0; i != 16; ++i) {
        futures[i] = std::async(std::launch::async, foo, i);
   }
   for (int i=0; i!=16; ++i) {
        futures[i].wait();
   }
}

[kripken]

@omnisip

Trying to get what you wrote to compile, I hit

a.cpp:10:15: error: use of class template 'future' requires template arguments
   std::array<future, 16> futures;

It can save a lot of time to provide already-runnable testcases.

[omnisip]

@omnisip

Trying to get what you wrote to compile, I hit

a.cpp:10:15: error: use of class template 'future' requires template arguments
   std::array<future, 16> futures;

It can save a lot of time to provide already-runnable testcases.

future<int> sorry.

Give me a few minutes and I'll push a godbolt for you.

[omnisip]

https://godbolt.org/z/j31v84

em++ -g3 -O3 -s PTHREAD_POOL_SIZE=16 -s USE_PTHREADS=1 -s MODULARIZE=1 -s ALLOW_MEMORY_GROWTH=1 -s ASSERTIONS=1 -msimd128 -mavx -s EXPORTED_FUNCTIONS='[ _malloc, _free, _functionCalledFromNodeJs ]' -s EXPORTED_RUNTIME_METHODS='[ccall, cwrap]' -s 'EXPORT_NAME=threadTest' -I. -o threadTest.js threadTest.cpp

dan@dl360:~/applications/test-rgb2y$ cat testrunner-thread.js 
let loader = require('./threadTest');
loader().then(m => { 
        m.ccall('functionCalledFromNodeJs','void',[],[]);
})

/usr/bin/node --experimental-wasm-simd --experimental-wasm-modules --experimental-wasm-threads --experimental-wasm-bulk-memory ./testrunner-thread.js

[omnisip]

if you change the PTHREAD_POOL_SIZE to 32, it will succeed, but will fail at 16.

Code inlined for convenience:

#include <array>
#include <future>


int foo (int p) {
return p;
}

extern "C"
void functionCalledFromNodeJs() {
   std::array<std::future<int>, 16> futures;
 
// first pass
   for (int i = 0; i != 16; ++i) {
        futures[i] = std::async(std::launch::async, foo, i);
   }
   for (int i=0; i!=16; ++i) {
        futures[i].wait();
   }
    
// second pass
   for (int i = 0; i != 16; ++i) {
        futures[i] = std::async(std::launch::async, foo, i);
   }
   for (int i=0; i!=16; ++i) {
        futures[i].wait();
   }
}

[kripken]

Thanks @omnisip !

It looks like what happens here is that when a worker exits, it postMessages the main thread to tell it that it can be reused ('exit'). But in this code, the main thread never exits to the event loop, it just constantly runs, so it never gets that.

It may be possible to avoid this problem by sending the message using the C queues, or some other mechanism that avoids the JS event loop.

[kripken]

I'm not sure how robust that can be. In general, using -s PROXY_TO_PTHREAD can avoid this entire class of problems - I verified it can run this testcase ok.

[omnisip]

With proxy to pthread, I tried it before filing this issue. Is there a new version that has a fix or something?

[kripken]

@omnisip I don't remember a related fix. But please try it on the latest version. If it doesn't work for you with PROXY_TO_PTHREAD, that could be a separate issue.

[omnisip]

Retry the sample with PTHREAD_POOL_SIZE=4 and PROXY_TO_PTHREAD=1. It will fail. I think the behavior should be that it restores the threads after completion -- so even if it's not an ideal pool size, it should handle them in order.