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

[WIP] Expose gas metering in the API #68

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ethanfrey
Copy link
Contributor

@ethanfrey ethanfrey commented Aug 30, 2019

This is based on wasmerio/wasmer#736 using an *.so with --features metering enabled.

Do not merge until that above PR has made it to master (or tag).

This seeks to expose a few more calls in the API:

  • CompileWithLimit
  • InstanceGetPoints (TODO)
  • InstanceSetPoints (TODO)

Each calls the new functions on the upstream metering branch. (Note if metering feature is disabled in rust build, these exist but do the same as Compile and no op.

There are some updates to the rust build and the current libwasmer_runtime_c_api.so file is now the metering branch with metering enabled.

To demo this:

just build
# relevant tests here
go test ./wasmer/test -run Limit -v
# all tests
just test

The limit tests fail with one known issue i have seen in rust code as well (calling a 1 op contract when gas limit was exceeded doesn't fail... it must use at least 8 gas to trigger a failure).

Only the libwasmer_runtime_c_api.so file is updated. If you use a different architecture, or what to build it all from source, try just rust-install before the above build/test cycle.

Open issues:

  • I copied over wasmer.h from runtime-c-api output, which I think is proper.
  • I did manually update bridge.go which is likely auto-generated.
  • Some tests (export/import) fail.

@Hywan Hywan self-assigned this Sep 6, 2019
@Hywan Hywan changed the title Expose gas metering in the API [WIP] Expose gas metering in the API Sep 6, 2019
@Hywan Hywan added 🎉 enhancement New feature or request 📚 documentation Do you like to read? 📦 component-extension About the Go extension 📦 component-runtime About the Wasm runtime 🧪 tests I love tests labels Sep 6, 2019
@Hywan
Copy link
Contributor

Hywan commented Sep 6, 2019

Thanks for the PR!

I copied over wasmer.h from runtime-c-api output, which I think is proper.

It is!

I did manually update bridge.go which is likely auto-generated.

This file isn't auto-generated. I wrote it by hand and is manually maintained.

Some tests (export/import) fail.

That's OK, we will iterate on this.

@ethanfrey
Copy link
Contributor Author

@Hywan Okay, so it seems I did the wiring properly.

Can you let me know what steps are needed both here and on the linked PR to proceed?

@AdamSLevy
Copy link
Contributor

I am also interested in this feature for my project and would be happy to help with any remaining tasks if they can be identified.

@Hywan
Copy link
Contributor

Hywan commented Nov 18, 2019

I've to catch up on this soon.

@Hywan
Copy link
Contributor

Hywan commented Nov 18, 2019

@ethanfrey Can you update your PR to master please? If no, tell me, I'll.

@ethanfrey
Copy link
Contributor Author

@Hywan You can close this PR, or take it over. It was frozen so long, I ended up building my own bindings rust -> go.

There were some open questions, such as how to enable it, and if it forces singlepass backend even for non-metered usage if the metering option is enabled compile time, or using different backends based on metered/non-metered usage. My impression is this opened up some cans of worms, and the ideal solution was to wait for metering support with cranelift backend, then unify everything this that default. I felt this was blocked as there was no ideal solution to enable metering without hampering the non-metered usage. If someone wants to take over this issue, I am happy to write out my learnings.

If you want to look at my rust->go binding, you can. They are quite opinionated, only for the usages of wasmer vm that I need in my project, so probably not so useful for others, but maybe they are useful as an example if you want to put most of the business logic in rust and only expose a small API to Go (you are welcome to fork and modify). https://github.com/confio/go-cosmwasm/

@AdamSLevy
Copy link
Contributor

@ethanfrey @Hywan I am interested in picking this up. I am working on a project that needs metering and the singlepass backend compiler. I'm currently reviewing the work already done in wasmerio/wasmer#736 but would welcome any guidance as I'm very new to Rust, but familiar with Go and C, and CGo.

@ethanfrey
Copy link
Contributor Author

The go bindings should not be too hard to refine once the wasmer runtime adds support. Note that the only real conflict is wasmer/wasmer.h (and rebuild the so file with an updated Cargo).

The blocking issue is reflected in this comment: wasmerio/wasmer#736 (comment)

IMO, this requires a clear decision from wasmer maintainers before proceeding. You can also just use my PR branches if you want, along with the provided features flags. It works, even if a bit out-of-date version of wasmer. But I only suggest that for prototypes, this must be stabilized somehow for any production build.

@AdamSLevy
Copy link
Contributor

AdamSLevy commented Nov 19, 2019

@ethanfrey Thanks for the input.

As I understand it, one hesitation to adding metering support to this library is that currently this package uses the cranelift backend, which currently has metering issues. The other backends: singlepass and llvm, support metering but are not necessarily good defaults for most applications. To expose the backend as a runtime option would require all backends to be compiled and statically linked into all applications that use this package.

One solution is to compile 3 separate C API .so, one for each backend, and use go's build tags to control which one to link against. This can be designed so that projects with no build tags default to the current cranelift backend.

For metering, the APIs could also be contingent on the build tags, so that they aren't available at compile time if the cranelift backend is used. This restriction can later be removed once the cranelift backend's issue with metering is resolved.

@Hywan What do you think of an approach using Go's build tags like this? If you are interested I can go ahead an open up a PR demonstrating it.

@ethanfrey
Copy link
Contributor Author

Interesting idea. It could also just be cranelift and singlepass to start. LLVM is a bit more complicated to build (especially if you try to cross-compile for multiple platforms). You need a matrix of libs, wasmer-{singlepass,cranelift,llvm?}.{so,dll,dylib}

But in general, it sounds fine. I'm not sure how powerful go's build flags are, but if you can make it work, I think it is a good idea (and just make your own pr deprecating this, as that is a lot more work, not just a small patch).

@Hywan Hywan added this to 🔍 Review in Kanban Dec 4, 2019
@AdamSLevy
Copy link
Contributor

AdamSLevy commented Dec 6, 2019

Basically go's build flags can let you control conditional compilation at the file level. If we move this line which controls linking to separate files with the appropriate suffix, like link_{backend}.go, then we can use these tags to conditionally link against various backends.

https://github.com/wasmerio/go-ext-wasm/blob/bf837c2dd588e346ecafe9a6c1cc956801d04626/wasmer/bridge.go#L3

It's possible that this could also be designed to allow use of multiple backends within a single Go application. Basically some sort of Backend variable could serve as a selector and could be added to the library within these backend specific go files. If a program wants to use more than one backend, their application will need to declare all of the backends in build tags in order for it to be visible in their program. But I'm not sure the C API would allow this currently.

Selecting a backend is surely possible with build tags as this is how go handles architecture specific code.

I am 90% sure this is possible. I will play around with this sometime in the next few weeks probably.

@sorawit
Copy link

sorawit commented Mar 21, 2020

Hi @AdamSLevy @Hywan. What's the status of this ticket on your priority list? Are you guys planning to expose metering API to Go soon? We are looking into how we can run Wasm code in Go with metering and would like to see if it's possible with Wasmer. Thanks!

@ethanfrey
Copy link
Contributor Author

@sorawit I abandoned this PR as there were a number of issues on how to expose different configurations via one dll. I ended up building most of my low-level logic in Rust and wrote my own binding library for my app: https://github.com/CosmWasm/go-cosmwasm.

Frankly if this use case is important to you I would suggest forking either building your own binding, or forking both go-ext-wasm and wasmer (the runtime_c_api package) to add that. This code can be used as a guideline but is extremely outdated and wasmer development moves quite fast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 component-extension About the Go extension 📦 component-runtime About the Wasm runtime 📚 documentation Do you like to read? 🎉 enhancement New feature or request 🧪 tests I love tests
Projects
Kanban
  
🔍 Review
Development

Successfully merging this pull request may close these issues.

None yet

4 participants