-
Notifications
You must be signed in to change notification settings - Fork 5.1k
wasm: enable wamr #40432
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
wasm: enable wamr #40432
Conversation
Change-Id: I06eb08f6ed88cbe9a0acdeccc1a16c588327f3d6 Signed-off-by: Kuat Yessenov <[email protected]>
Change-Id: I9ff5d31b21c94af188563b45051b2d61b5f1ca87
Change-Id: I98ab99aadc616438de547afe3d4642c0847fb032 Signed-off-by: Kuat Yessenov <[email protected]>
Change-Id: I3dcfe391e92e132789688df3fa7cc5127da9509c Signed-off-by: Kuat Yessenov <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
High-level question:
Will it make sense to choose a single engine that will be included by default?
Assigning Yan and Mike to hear their thoughts.
/assign @yanavlasov @mpwarres
|
We had an offline discussion to support WAMR. We still need to make a choice on what flavor of WAMR (fast interpreter and JIT are not compatible, it's one or the other; Fast JIT is not compatible with Aarch64). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/wait-any
|
|
||
| * `v8` (the default included engine) | ||
| * `wamr` | ||
| * `v8` (default included engine) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we decided to exclude v8 from the default build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If V8 is not part of the default build, which builds would it be included in, and would those be covered by test runs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the default build is similar to deprecating the extensions, so we needs a heads up IMO and could keep both for a release. Or we could just drop it. @mpwarres should we try JIT WAMR instead of interpreter one? It seems like you can't have both of them so we should make a choice now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC the issue with JIT WAMR was that it is not supported for Arm. Would it be feasible to have the amd64 build use WAMR JIT but use the interpreter for arm64?
Sorry for the dumb question but I am still unclear whether we will still have CI test runs exercising V8 after it is dropped from the default build. IMO continuing to run Envoy WasmFilter tests with V8 is desirable to avoid bitrot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently the compile-time-options ci test against the non-default wasm engines
|
needs main merge /wait |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Commit Message: Add Wamr runtime to the default build.
Additional Description: Fast interpreter mode is a reasonable tradeoff - at a very rough estimate, it is ~3x lighter in memory use (but also ~3x slower in CPU).
Risk Level: low
Testing: no (tested upstream)
Docs Changes: yes
Release Notes: yes