Skip to content

Conversation

vladmandic
Copy link
Contributor

this pr mirrors the fix from #16450 which solves the issue for library_pthread.js to fix worker.js as well
discussed in issue #17844

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having test here would be good, although it might be fiddly to test for the absence of log messaged.

I would like to see an example of an app that uses custom messages in a worker. Do you think you could add something the use EM_ASM or EM_JS to do such things in a thread?

@vladmandic
Copy link
Contributor Author

Having test here would be good, although it might be fiddly to test for the absence of log messaged.

I would like to see an example of an app that uses custom messages in a worker. Do you think you could add something the use EM_ASM or EM_JS to do such things in a thread?

The use-case for TFJS is really to work-around Chrome's built-in throttling

To resolve an async function, it needs to internally check if result if available so it does frequent polling
But if setTimeout nesting level is greater than 5 and timeout is less than 4ms, timeout will be clamped to 4ms
Which means that in some cases await function() will have a minimum resolve time of 4ms even if its done much faster

Obviously thats not a desired behavior as it has quite a performance impact as for example function runs on each frame in real-time video input processing - its significantly reducing overall FPS

In those cases, workaround is to use postMessage and resolve in onmessage handler instead

I'll try to think of a simple reproduction without going too deep in TFJS library internals, but it may be tricky

@sbc100
Copy link
Collaborator

sbc100 commented Sep 14, 2022

Having test here would be good, although it might be fiddly to test for the absence of log messaged.
I would like to see an example of an app that uses custom messages in a worker. Do you think you could add something the use EM_ASM or EM_JS to do such things in a thread?

The use-case for TFJS is really to work-around Chrome's built-in throttling

To resolve an async function, it needs to internally check if result if available so it does frequent polling But if setTimeout nesting level is greater than 5 and timeout is less than 4ms, timeout will be clamped to 4ms Which means that in some cases await function() will have a minimum resolve time of 4ms even if its done much faster

Obviously thats not a desired behavior as it has quite a performance impact as for example function runs on each frame in real-time video input processing - its significantly reducing overall FPS

In those cases, workaround is to use postMessage and resolve in onmessage handler instead

I'll try to think of a simple reproduction without going too deep in TFJS library internals, but it may be tricky

Why does the example need to be complex? Why can't we just have a thread that sends a message using windows.postMessage and shows that it can be received by a custom message handler? (and ideally without any errors on stderr).

@sbc100
Copy link
Collaborator

sbc100 commented Sep 14, 2022

I think because this is such simple fix, and the test is tricky we can probably just land this change as if you prefer.

@kripken kripken merged commit c5110bf into emscripten-core:main Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants