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 ResourceLimiter to wasmi::Store #728

Closed
8 of 10 tasks
Robbepop opened this issue May 29, 2023 · 4 comments · Fixed by #737
Closed
8 of 10 tasks

Add ResourceLimiter to wasmi::Store #728

Robbepop opened this issue May 29, 2023 · 4 comments · Fixed by #737

Comments

@Robbepop
Copy link
Member

Robbepop commented May 29, 2023

As requested in this comment some wasmi users could profit from having access to a Wasmtime style ResourceLimiter API.

Since wasmi tries to mirror the Wasmtime API we should and could mirror this API from Wasmtime as well. I think it would be a nice addition, however, special care has to be taken that translation and execution performance do not regress.

TODOs

  • Implementation
  • Testing & QA
    • Add tests to verify that the implemented functionality works as intended.
    • Add proper docs to the feature as well as some doc example.
    • Validate with benchmarks that using this feature does not disrupt wasmi performance too much.
    • Validate with benchmarks that not using this feature does not disrupt wasmi performance significantly.

Note: Wasmtime's epoch_interruption is not needed since wasmi does not (yet) support async execution.

@jayz22
Copy link

jayz22 commented May 31, 2023

@Robbepop do you have any rough estimation on the timeline? I'm definitely very interested in this feature and would be willing to help if needed.

@Robbepop
Copy link
Member Author

Sorry for my late response!

At the moment I am working on #729 which takes a big chunk of my time. I thought that this issue might be a perfect candidate for external contributions since it should be a rather local change. If it is extremely pressing we can probably work something out together. Shouldn't be a big task from what I see so far.

@graydon
Copy link
Contributor

graydon commented Jun 27, 2023

@Robbepop ok if you're comfortable having us implement this, we're happy to direct our efforts to it. It looks like it ought to be relatively straightforward.

@Robbepop
Copy link
Member Author

Robbepop commented Jun 27, 2023

@graydon Yes I am very comfortable with this being contributed externally. From the top of my head this should be fairly straight forward to implement. The worst thing that could happen is that we have to figure out performance issues reported by the CI since wasmi is kinda sensitive to changes in its executor.

edit: I updated this issue's TODO list to properly reflect the required work items. The ResourceLimits and ResourceLimitsBuilder types are not needed for an MVP implementation but nice to have for convenience.

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 a pull request may close this issue.

3 participants