no_dyncall_garbage #4894
no_dyncall_garbage #4894
Conversation
| @@ -4192,7 +4192,7 @@ LibraryManager.library = { | |||
| var trace = _emscripten_get_callstack_js(); | |||
| var parts = trace.split('\n'); | |||
| for (var i = 0; i < parts.length; i++) { | |||
| var ret = Runtime.dynCall('iii', [0, arg]); | |||
| var ret = Runtime.dynCall_iii(func, 0, arg); | |||
There was a problem hiding this comment.
This call site was missing reference to 'func' in the call, so it probably didn't work at all in the first place?
There was a problem hiding this comment.
Yeah, this was added for Rust but we never found a good way to test it. I wouldn't worry about it.
|
Hmm, I do see we don't generate a 'dynCall_v' function for example in some cases. I wonder for this to work at the moment we'll need to have a C side function pointer call with these signatures? If so, I wonder if we could somehow use the 'foo__deps: ['xxx']' arch to signal the generation of the required dynCalls? |
| @@ -478,7 +478,7 @@ function emscripten_start_fetch(fetch, successcb, errorcb, progresscb) { | |||
| #if FETCH_DEBUG | |||
| console.log('fetch: operation success. e: ' + e); | |||
| #endif | |||
| if (onsuccess && Runtime.dynCall) Runtime.dynCall('vi', onsuccess, [fetch]); | |||
| if (onsuccess && Runtime.dynCall) Runtime.dynCall_vi(onsuccess, fetch); | |||
There was a problem hiding this comment.
instead of Runtime.dynCall_vi this (and the rest) should be Module['dynCall_vi']
(do we also copy them to Runtime? I don't think so, and looking in the code I don't see that either)
There was a problem hiding this comment.
Ohh right, confused 'Runtime' vs 'Module' here, updated PR.
This PR shouldn't change anything there - if something fails with this PR, it would have failed before. I don't remember all the details about how we decide to generate those, but given we're not aware of bugs in this area, I wouldn't worry about it. |
…me.dynCall('signatureString', func, [args]) in favor of the static form Runtime.dynCall_signature(func, args) which is faster and produces no garbage.
juj
force-pushed
the
no_dyncall_garbage
branch
from
d04e001 to
7ee40f3
Compare
|
Thanks, lgtm. I would do some local testing before pushing though (or watch bots carefully). So much garbage saved... |
|
Ran the 'browser' suite to verify, also going to try out on a real world test case before landing. |
|
Verified this in real world test case, all looks good now. |
Fix function pointer call in _Unwind_Backtrace(). Remove use of Runtime.dynCall('signatureString', func, [args]) in favor of the static form Runtime.dynCall_signature(func, args) which is faster and produces no garbage.