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

Async export support #112

Closed
guybedford opened this issue May 30, 2024 · 5 comments
Closed

Async export support #112

guybedford opened this issue May 30, 2024 · 5 comments

Comments

@guybedford
Copy link
Collaborator

When implementing sync component model functions as async exports in JS (functions returning a promise), we should integrate with the event loop by:

  • Detecting if a promise is returned in the bindgen, and treating this as a special case which awaits the promise to get the result
  • In this same code, to have a function in the JS bindgen that performs increase_event_loop_interest() before awaiting the promise and decrease_event_loop_interest() on the promise then and catch handlers.
  • Creating a JS function in ComponentizeJS available to the bindgen which reflects these functions to StarlingMonkey's implementation of them
  • Calling run_event_loop in the main export function wrapper if there is event loop interest to resolve the return value

This is just a rough outline off the top of my head but hopefully it helps.

@karthik2804
Copy link
Contributor

I do not understand the internals of the event loop yet. I just wanted to note that this seems like a regression in #111.

The behavior on the latest release seems to be the expected behavior where the guest seems to run until all the async tasks complete.

@guybedford
Copy link
Collaborator Author

I think it's actually a bug that it was working before actually as the event loop architecture in StarlingMonkey has since changed. The technical counter-argument is that draining the event loop entirely risks allowing setInterval(() => {}) to saturate the compute. Exact event loop discussion is still active in bytecodealliance/StarlingMonkey#56.

@tschneidereit
Copy link
Member

I think this makes sense to do, yes. An additional or alternative thing we could consider is to expose a way for embedders to spin the event loop explicitly, or at least so explicitly register interest.

@noise64
Copy link

noise64 commented Jun 19, 2024

We (https://github.com/golemcloud/) are quite excited about this and the fetch support, so as an experiment with this idea we merged the current pending related PRs into forks (both on StarlingMonkey and ComponentizeJs), and then moved around a bit the interest markers and exposed "runEventLoop". This way after making a fetch request we can call explicitly the event loop, and by the time it returns, the fetch and related promises are nicely resolved and do work (so we can return them synchronously), which is quite promising, and would be something that we would be happy to live with until this topic evolves more.

On the other hand, repeated (10-100) calls to fetch in the same exported call and / or specific payload sizes (depending on the specific code) can trigger errors in the post_call phase, specifically in cabi_free. With node it results in "RuntimeError: memory access out of bounds" and similar errors are happening with wasmtime.

I do see that there is also a probably related issue on how the current "gc" is implemented (#1), and probably this is a bit more complicated topic then i think, but wanted to ask if you might be able to provide any pointers on how one should continue debugging such errors or whether this is something that expected for now or if not, then it could be also added as a test case to the fetch ones.

@guybedford
Copy link
Collaborator Author

This is now supported in #131.

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

No branches or pull requests

4 participants