Remove ENVIRONMENT_IS_PTHREAD variable.

[juj]

Remove ENVIRONMENT_IS_PTHREAD variable. The existence of this variable predates to the time when pthreads still supported --proxy-to-worker mode (and --proxy-to-pthread did not exist). After --proxy-to-pthread has proven itself as a more usable method and USE_PTHREADS + --proxy-to-worker was deprecated, there is no longer need to distinguish between "am I a worker" or "am I a pthread", as all workers are pthreads, and the scenario "worker that is the proxied main thread so not a pthread" no longer happens.

[sbc100]

I'm a huge fan of this kind of change!

Just a quick question: Is there not a use case for single threaded worker that (built without shared memory) that still proxies class to the main thread? Or is the proxying itself dependend on shared memory making this use case impossible?

Also, probably worth a ChangeLog entry since its technically user-visible.

[kripken]

What happens if the application is started in a worker? Then ENVIRONMENT_IS_WORKER is true on the main thread as well as in the workers that are created for pthreads, it seems.

[juj]

Do you mean the --proxy-to-pthread option, where the application main() function is run in a Worker? Then yes, ENVIRONMENT_IS_WORKER will be true in that main() function (as per design - ENVIRONMENT_IS_PTHREAD was also true in that case).

[kripken]

No, sorry, I meant when using pthreads normally (not --proxy-to-pthread). If an application using pthreads is instantiated in a worker, then this PR makes us lose the ability to differentiate the main thread from the others, doesn't it? I may be missing something.

[juj]

Such run configuration has never been supported or tested (and has never worked, I'd be really surprised if it did). I.e. one cannot do a build with -s USE_PTHREADS=1 and manually run the resulting a.js code in a Worker.

Also one cannot do a -s USE_PTHREADS=1 --proxy-to-pthread build and launch the resulting a.js code in a Worker.

That being said, if someone was launching -s USE_PTHREADS=1 builds in a worker and it did end up working, modulo all the immediate limitations (cannot access DOM, cannot do WebGL, requires support for nested Workers - (did Chrome add that?), cannot do requestAnimationFrame), then we indeed would need to retain a ENVIRONMENT_IS_PTHREAD flag. If that is a configuration that should be supported, that warrants adding testing coverage - in this form this change passes the test suite.

[kripken]

Very good point that we were missing testing! I opened #10036 now to fix that.

I think this has been supported, and documented even, but the testing relied on PROXY_TO_PTHREAD and --proxy-to-worker for convenience, and so we never actually tested the main application thread being in a worker.

I think it's an important use case to support, for pure computational code. Imagine a chess engine or a physics engine compiled to wasm, then maybe the JS application using it happens to be itself in a worker.

Nothing in our pthreads support code should depend on being on the main browser thread, I don't think - in fact we added node.js pthreads support, so the main thread can even be outside the browser. That PR passes for me locally, hopefully it passes on CI too - if not, I think we should fix that.

[kripken]

Oh, also good point about nested workers in Chrome - that was actually fixed a little over a year ago, in Chrome 69 it appears: https://www.chromestatus.com/feature/6080438103703552 Sadly it says Safari doesn't support it yet 😢

[juj]

I think this has been supported

No, for a long long time such scenario has not been supported, primarily due to nested Workers support, that only worked in Firefox. If someone has been doing this, they have been lucky on their own that it has happened to work out. Certainly all development on threads runtime that I have done has not assumed this to be a setup that works.

the testing relied on PROXY_TO_PTHREAD and --proxy-to-worker for convenience

For a very brief period of time, I was developing USE_PTHREADS and --proxy-to-worker support to work at the same time. In this mode, to avoid nested workers case, the idea was to route threads creating from the main() Worker to main browser thread. But the big problem was that it would create a nasty communication ping-ponging problem where a pthread might need to pass data to main() thread, and would need to postMessage to main UI thread, and that would need to postMessage to the main() thread. Such ping-ponging was too bad for performance; and I looked at two approaches to remedy: MessagePorts, and then --proxy-to-pthread. --proxy-to-pthread proved much easier, so I dropped the nascent -s USE_PTHREADS + --proxy-to-worker development. This was referred to in --proxy-to-pthread PR.

That is why I say I'm surprised if someone is actually running this kind of USE_PTHREADS + manually-running-in-a-Worker scenario and it works for them.

[kripken]

Interesting, I had assumed it worked when I rewrote the docs earlier this year. Turns out that was false, as you say, but it was almost true - as mentioned in #10036 a one-line fix makes it work (everywhere but Safari). So it didn't work for anyone, as you said, but we can fix it.

I do remember some of that proxy-to-pthread history now that you mention it, thanks!

I think we should support pthreads programs in workers, so I am in favor of #10036, and not removing ENVIRONMENT_IS_PTHREAD.

[juj]

As per #10036, adding it as a supported configuration to be able to run pthreads-enabled code in a Worker, so I'll close this out.