Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add simple repro test for async setInterruptHandler #90

Closed
wants to merge 1 commit into from

Conversation

swimmadude66
Copy link
Contributor

@swimmadude66 swimmadude66 commented Dec 13, 2022

Repro test for #84

Added a basic test of executing sync code and can verify that the interrupt is called once in that case, but as soon as async code is run, we don't even get the first call to the interrupt handler.

I added a couple of global functions which we use in our project in case those are the source of the issue. They work pretty well aside from apparently disabling interrupt logic 😅

The first is "$delay" which is just a promise-ified timeout for easy async testing. The second is a "done" method, which wraps a new VM Promise and allows us to await deferred.settled outside the VM before trying to read the results

@justjake
Copy link
Owner

The interrupt check is intended to limit the CPU time of a runtime. It is executed every 10,000 instructions by the QuickJS runtime. If the runtime is not executing instructions, the interrupt handler will not be called. I think what's likely here is that you are suspending execution inside the runtime to wait on external promises; during that time, there is no execution occurring inside the runtime, so the interrupt handler is never called.

Also, note that the "async" versions of QuickJSRuntime and QuickJSContext as well as evalCodeAsync are only useful if you need to call async host functions as though they are synchronous from the perspective of guest code inside the VM. You are not using that feature in these tests, so double-check if using the asyncify version is necessary. It comes with a substantial performance impact.

@swimmadude66
Copy link
Contributor Author

Oh interesting! In our primary usecase we have a lot of async host code, but I believe its all wrapped and async in the VM as well. Trying out moving our usecase to the non-async WASMModule now, and it appears to work and work faster, so thank you for that tip!

However, even with that change, the interruptHandler is never called for our code. The 10k cycle interval for calling interrupt is interesting. Most of the time, our long-running functions are going to be waiting on a promise to resolve, which it sounds like wouldn't cause regular calls to the interruptHandler code. I've written a JS-language poller which calls a handler on the host regularly, but I don't have a way to force an interrupt from that, do I?

Is there a way to add an explicit interruptRuntime method then?

@justjake
Copy link
Owner

justjake commented Dec 14, 2022

The purpose of the existing interruptHandler is to regain control of the CPU, which is not possible in any other way.

I don’t think any API extensions are needed to ”interrupt” async behavior - while the guest is waiting on a promise, the host has full CPU control and can do whatever it wants - including modifying VM globals, or choosing to never call back into the guest again.

You can have your promise-returning functions reject after a timeout is reached or some other interrupt condition occurs. You can use Promise.race([userPromise, deadline]) when your host code waits on a guest promise. You can implement your “interrupt handler” wherever you’re currently calling “executePendingJobs”, and decide not to call executePendingJobs.

@swimmadude66
Copy link
Contributor Author

We've decided to go another way in our use-case, so I'm gonna close this for now

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.

2 participants