Fix closure errors with MINIMAL_RUNTIME + USE_PTHREADS. NFC. #14936
Conversation
sbc100
force-pushed
the
minimal_runtime_pthread_closure
branch
from
cccf6d3 to
1cebd87
Compare
| @@ -108,7 +108,7 @@ if (!ENVIRONMENT_IS_PTHREAD) { | |||
| #if USE_PTHREADS | |||
| } else { | |||
| #if MODULARIZE | |||
| updateGlobalBufferAndViews({{{EXPORT_NAME}}}.buffer); | |||
| updateGlobalBufferAndViews(Module.buffer); | |||
There was a problem hiding this comment.
Without this it fails closure compiler with MyModule not defined. I think this is because closure runs before modularization so at the point of closure compiler MyModule (the EXPORT_NAME) does not exist.
However Module.buffer works just fine here because the modularized code starts with
function(MyModule) {
MyModule = MyModule || {};
var Module = MyModule;
There was a problem hiding this comment.
I see, sounds good. And closure will remove the extra var anyhow.
sbc100
force-pushed
the
minimal_runtime_pthread_closure
branch
from
1cebd87 to
47f0c92
Compare
| @@ -1884,6 +1884,7 @@ def default_setting(name, new_default): | |||
| '_emscripten_tls_init', | |||
| '_pthread_self', | |||
| '_pthread_testcancel', | |||
| '_exit', | |||
There was a problem hiding this comment.
Does adding to this list pin down the function from being DCEd if not used? exit() is not used always in pthreads builds?
There was a problem hiding this comment.
Yes, this entire like of functions cannot be DCEd in USE_PTHREADS builds.
However _exit is not special here. Just like all the other functions here is used by library_ptheads.js in general, but might be used in a given application. For example, imagine an application that builds with USE_PTHREADS but never actually creates in any threads.. all of the functions in this list would essentially be unused but also prevented from DCE.
I guess we want some other notion of EXPORTED_FUNCTIONS that also allows for DCE, but I don't know if that exists.
Perhaps we could add something like EXPORTED_FUNCTIONS_FOR_POSSIBLE_INTERNAL_JS_USAGE_ONLY?
There was a problem hiding this comment.
Or maybe just POSSIBLY_USED_FUNCTIONS?
There was a problem hiding this comment.
The reference to _exit is from the exitProcess handling code, and I wonder if we could if that out when EXIT_RUNTIME is not set... that would be a good solution I think. On the other hand the proc_exit library functions (of which exit is an alias) is just a single line of code on MINIMAL_RUNTIME and given that USE_PTHREADS brings in many hundreds of extra lines of code (IIUC) it maybe not that urgent to fix?
There was a problem hiding this comment.
Oh, I think I wrong, please ignore those previous replies.
Adding to EXPORTED_FUNCTIONS does not prevent DCE, because we don't consider them USER_EXPORTED_FUNCTIONS. Only USER_EXPORTED_FUNCTIONS are prevented from DCE.
No description provided.