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 support for gas metering to cranelift backend #819

Closed
ethanfrey opened this issue Sep 20, 2019 · 4 comments
Closed

Add support for gas metering to cranelift backend #819

ethanfrey opened this issue Sep 20, 2019 · 4 comments
Labels
🎉 enhancement New feature!

Comments

@ethanfrey
Copy link
Contributor

Thanks for proposing a new feature!

Motivation

As I worked to expose gas metering in the C API, I discovered that it was not supported with the default cranelift backend. The wasmer team assured me this is a known issue and on their roadmap.

It would be nice to enable metering without having to switch backends (unless one wants to), and also make less headache in configuration. However, this is not urgent. Mainly I wanted an issue to track progress.

Proposed solution

Support for for StreamingCompiler with middleware when using wasmer_clif_backend::CraneliftModuleCodeGenerator.

This can be easily tested on the #736 branch (or master when merged) by going into runtime-c-api package and executing cargo test --features metering.

Alternatives

Officially mark that middleware is not supported with cranelift. But I think this feature is desired, just not enough devs to implement it.

Additional context

Add any other context or screenshots about the feature request here.

@syrusakbary
Copy link
Member

Awesome. Thanks for creating the issue so we can keep track of this over time :)

@nlewycky
Copy link
Contributor

@MarkMcCaskey is implementing the alternative suggestion in #817 now .

I think there's two ways to handle this:
a) We replace the cranelift-wasm crate with our own implementation. This allows us to fix this bug by permitting access to the VM internals section where we keep the counters for metering. This has other benefits, such as decoupling our cranelift and wasmparser.rs version upgrades.
b) We write a wasm-->wasm rewriter that adds metering to any wasm code keeping the result as valid, generic, wasm code. This means we couldn't use our VM internals to keep count, but it would work with all backends, including other VMs if we wrote the modified wasm out to disk.

@syrusakbary
Copy link
Member

In the refactor, all compilers will support any kind of middleware, including metering.

Will close this issue once we merge the refactor into master.

@syrusakbary
Copy link
Member

Closing this issue in favor of #1418

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature!
Projects
None yet
Development

No branches or pull requests

3 participants