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 WebAssembly Micro Runtime (WAMR). #142

Merged
merged 20 commits into from
May 7, 2021

Conversation

leyao-daily
Copy link
Contributor

Add WAMR support

@leyao-daily leyao-daily changed the title Add Webassembly micro runtime support [WIP] Add Webassembly micro runtime support Mar 18, 2021
@leyao-daily
Copy link
Contributor Author

@googlebot I signed it!

@leyao-daily
Copy link
Contributor Author

@googlebot I signed it!

@leyao-daily
Copy link
Contributor Author

@googlebot I fixed it.

@google-cla
Copy link

google-cla bot commented Mar 22, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@PiotrSikora
Copy link
Member

PiotrSikora commented Mar 23, 2021

@googlebot I fixed it.

@leyao-daily one of your commits has a line saying Co-authored-by: Liang He <[email protected]>, and we don't have signed CLA for that person.

@leyao-daily
Copy link
Contributor Author

@googlebot I fixed it.

@leyao-daily one of your commits has a line saying Co-authored-by: Liang He <[email protected]>, and we don't have signed CLA for that person.

Okay, I will remind him.

@google-cla
Copy link

google-cla bot commented Mar 23, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@mathetake
Copy link
Contributor

mathetake commented Mar 24, 2021

I believe you need to put

 #if defined(WASM_WAMR) 
     } else if (runtime_ == "wamr") { 
       vm_ = proxy_wasm::createWAMRVm(); 
 #endif 

here,

and

#if defined(WASM_WAMR)
    "wamr",
#endif

here as well to run test properly.

@leyao-daily
Copy link
Contributor Author

@googlebot I fixed it.

@leyao-daily
Copy link
Contributor Author

@googlebot I consent.

@google-cla
Copy link

google-cla bot commented Mar 25, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@lum1n0us
Copy link
Contributor

@googlebot I consent.

@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@proxy-wasm proxy-wasm deleted a comment from google-cla bot Mar 25, 2021
@leyao-daily leyao-daily marked this pull request as ready for review April 6, 2021 07:42
Le Yao and others added 14 commits April 30, 2021 02:02
Build and testing runtime

Signed-off-by: Le Yao <[email protected]>
Add wamr runtime to testing, passed

Signed-off-by: Le Yao <[email protected]>
using a pre-release archive until passing integration

Signed-off-by: liam <[email protected]>
WAMR is commom WASM c/c++ API based, implenment same as wasitime

Co-authored-by: Liang He <[email protected]>
Signed-off-by: Le Yao <[email protected]>
Build and testing runtime

Signed-off-by: Le Yao <[email protected]>
INFO: Analyzed 9 targets (0 packages loaded, 0 targets configured).
INFO: Found 1 target and 8 test targets...
INFO: Elapsed time: 4.954s, Critical Path: 3.83s
INFO: 25 processes: 2 internal, 23 linux-sandbox.
INFO: Build completed successfully, 25 total actions
//test:context_test                                                      PASSED in 0.1s
//test:exports_test                                                      PASSED in 0.1s
//test:null_vm_test                                                      PASSED in 0.2s
//test:runtime_test                                                      PASSED in 0.1s
//test:shared_data                                                       PASSED in 0.1s
//test:shared_queue                                                      PASSED in 0.1s
//test:vm_id_handle                                                      PASSED in 0.1s
//test/common:bytecode_util_test                                         PASSED in 0.2s

Executed 8 out of 8 tests: 8 tests pass.
INFO: Build completed successfully, 25 total actions

Signed-off-by: Le Yao <[email protected]>
INFO: Analyzed 9 targets (0 packages loaded, 0 targets configured).
INFO: Found 1 target and 8 test targets...
INFO: Elapsed time: 4.541s, Critical Path: 3.48s
INFO: 20 processes: 2 internal, 18 linux-sandbox.
INFO: Build completed successfully, 20 total actions
//test:context_test                                                      PASSED in 0.1s
//test:exports_test                                                      PASSED in 0.1s
//test:null_vm_test                                                      PASSED in 0.1s
//test:runtime_test                                                      PASSED in 0.1s
//test:shared_data                                                       PASSED in 0.1s
//test:shared_queue                                                      PASSED in 0.1s
//test:vm_id_handle                                                      PASSED in 0.1s
//test/common:bytecode_util_test                                         PASSED in 0.1s

Executed 8 out of 8 tests: 8 tests pass.
INFO: Build completed successfully, 20 total actions

Signed-off-by: Le Yao <[email protected]>
Starting local Bazel server and connecting to it...
DEBUG: Rule 'proxy_wasm_cpp_host__wasmtime_c_api_macros__0_19_0' indicated that a canonical reproducible form can be obtained by modifying arguments shallow_since = "1617649269 -0700"
DEBUG: Repository proxy_wasm_cpp_host__wasmtime_c_api_macros__0_19_0 instantiated at:
  /root/proxy-wasm-cpp-host/WORKSPACE:9:33: in <toplevel>
  /root/.cache/bazel/_bazel_root/5d3cda4fc30ff6fff6d96ed8522a1d40/external/proxy_wasm_cpp_host/bazel/dependencies.bzl:20:44: in proxy_wasm_cpp_host_dependencies
  /root/.cache/bazel/_bazel_root/5d3cda4fc30ff6fff6d96ed8522a1d40/external/proxy_wasm_cpp_host/bazel/cargo/crates.bzl:724:10: in proxy_wasm_cpp_host_fetch_remote_crates
  /root/.cache/bazel/_bazel_root/5d3cda4fc30ff6fff6d96ed8522a1d40/external/bazel_tools/tools/build_defs/repo/utils.bzl:201:18: in maybe
Repository rule new_git_repository defined at:
  /root/.cache/bazel/_bazel_root/5d3cda4fc30ff6fff6d96ed8522a1d40/external/bazel_tools/tools/build_defs/repo/git.bzl:186:37: in <toplevel>
INFO: Analyzed 9 targets (131 packages loaded, 34267 targets configured).
INFO: Found 1 target and 8 test targets...
INFO: Elapsed time: 117.312s, Critical Path: 101.49s
INFO: 334 processes: 165 internal, 169 linux-sandbox.
INFO: Build completed successfully, 334 total actions
//test:context_test                                                      PASSED in 0.1s
//test:exports_test                                                      PASSED in 5.7s
//test:null_vm_test                                                      PASSED in 0.1s
//test:runtime_test                                                      PASSED in 2.0s
//test:shared_data                                                       PASSED in 0.1s
//test:shared_queue                                                      PASSED in 0.1s
//test:vm_id_handle                                                      PASSED in 0.1s
//test/common:bytecode_util_test                                         PASSED in 0.1s

Executed 8 out of 8 tests: 8 tests pass.
INFO: Build completed successfully, 334 total actions

Signed-off-by: Le Yao <[email protected]>
INFO: Analyzed 9 targets (5 packages loaded, 930 targets configured).
INFO: Found 1 target and 8 test targets...
INFO: Elapsed time: 5.281s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
//test:context_test                                             (cached) PASSED in 0.1s
//test:exports_test                                             (cached) PASSED in 5.7s
//test:null_vm_test                                             (cached) PASSED in 0.1s
//test:runtime_test                                             (cached) PASSED in 2.0s
//test:shared_data                                              (cached) PASSED in 0.1s
//test:shared_queue                                             (cached) PASSED in 0.1s
//test:vm_id_handle                                             (cached) PASSED in 0.1s
//test/common:bytecode_util_test                                (cached) PASSED in 0.1s

Executed 0 out of 8 tests: 8 tests pass.
INFO: Build completed successfully, 1 total action

Signed-off-by: Le Yao <[email protected]>
since `wasm_functype_new` will transfer data ownerships of both `params`
and `resutls` to the new `wasm_functype_t`, we should not
- delete `wasm_valtype_t` of `wasm_valtype_vec_t`
- use `wasm_functype_delete` to release members of `wasm_functype_t`

a good example will be found here:
[multi.c](https://github.com/WebAssembly/wasm-c-api/blob/master/example/multi.c)

another thing is the runtime() should return a string as same as the one in envoy.yaml

update wamr code release to 0.1-beta

Signed-off-by: liam <[email protected]>
To the latest official commit

Signed-off-by: Le Yao <[email protected]>
@leyao-daily leyao-daily requested a review from PiotrSikora April 30, 2021 02:10
},
lib_source = ":srcs",
out_shared_libs = ["libiwasm.so"],
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need LLVM dependency like in Envoy?

cmake(
name = "libiwasm",
cache_entries = {
"WAMR_BUILD_AOT": "0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please match it to options used in Envoy. At the very least, please add:

"WAMR_BUILD_INTERP": "1",
"WAMR_BUILD_JIT": "0",

or does JIT work already?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✋ just curious, I didn't find any WAVM related BUILD file. Does it mean WAVM can not be linked in this repo yet?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, currently you cannot build WAVM in this repo. We're slowly fixing this (see: #160), which is why it would be helpful if WAMR worked.

src/wamr/wamr.cc Outdated Show resolved Hide resolved
@leyao-daily
Copy link
Contributor Author

leyao-daily commented May 2, 2021 via email

Signed-off-by: Le Yao <[email protected]>
Copy link
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mathetake could you take a pass on this PR? I think it looks good, and tests in Envoy passed already (envoyproxy/envoy#16057).

I think the only thing missing right now is the LLVM dependency, but perhaps we could merge it as-is to unblock merge in Envoy, and we could fix any build issues as part of #160.

What do you think?

@mathetake
Copy link
Contributor

this looks good to me now and we could merge as-is since Envoy tests pass. And I will handle build/test issues here in #160 and subsequent PRs.

@PiotrSikora PiotrSikora changed the title Add Webassembly micro runtime support Add support for WebAssembly Micro Runtime (WAMR). May 7, 2021
Copy link
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot! @leyao-daily @lum1n0us please let me know if this is ready to be merged or if you still want to make some changes, thanks!

@leyao-daily
Copy link
Contributor Author

LGTM, thanks a lot! @leyao-daily @lum1n0us please let me know if this is ready to be merged or if you still want to make some changes, thanks!

Cool! We think it is ready to be merged now. Thanks a lot.

@PiotrSikora PiotrSikora merged commit e641ffa into proxy-wasm:master May 7, 2021
@PiotrSikora
Copy link
Member

Thanks!

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.

4 participants