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.

Sign up for GitHub

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

Jump to bottom

feat: support WasmEdge as an alternative engine #1

Draft
wants to merge 33 commits into
base: master
Choose a base branch
from
Draft

feat: support WasmEdge as an alternative engine #1

wants to merge 33 commits into from

Conversation

xxchan
Copy link
Owner

@xxchan xxchan commented Feb 26, 2023 •
edited
Loading

Test:

# prepare the .wasm file
cd smartmodule/examples && make build

# run test
cd ../..
source $HOME/.wasmedge/env
cargo test -p fluvio-smartengine --features wasmedge-engine


running 8 tests
test engine::wasmedge::transforms::filter_map::test::test_filter_map ... ok
test engine::wasmedge::transforms::aggregate::test::test_aggregate_with_initial ... ok
test engine::wasmedge::transforms::filter::test::test_filter_with_init_invalid_param ... ok
test engine::wasmedge::transforms::map::test::test_map ... ok
test engine::wasmedge::transforms::filter::test::test_filter ... ok
test engine::wasmedge::transforms::aggregate::test::test_aggregate_ok ... ok
test engine::wasmedge::transforms::filter::test::test_filter_with_init_ok ... ok
test engine::wasmedge::transforms::array_map::test::test_array_map ... ok

@xxchan xxchan force-pushed the xxchan/wasmedge branch 3 times, most recently from 11ed3bd to d76bbd8 Compare February 27, 2023 10:13
@xxchan xxchan mentioned this pull request Mar 10, 2023
Open
5 tasks
@xxchan xxchan force-pushed the xxchan/wasmedge branch 2 times, most recently from cc4fa2d to f468a3a Compare March 19, 2023 08:12
@xxchan xxchan force-pushed the xxchan/wasmedge branch 8 times, most recently from 0ad6295 to 1368090 Compare April 11, 2023 21:22
@xxchan xxchan force-pushed the xxchan/wasmedge branch 2 times, most recently from 95c20a2 to 9dfbb6b Compare April 15, 2023 12:49
@xxchan xxchan mentioned this pull request Apr 15, 2023
Closed
@juntao juntao mentioned this pull request Apr 29, 2023
Open
@xxchan
Copy link
Owner Author

xxchan commented Apr 29, 2023

WasmEdge support completed in 7924834. All original tests passed. (Most code & tests are shared by both engines!)

Keep working on

  • WasmEdge demo (e.g., TF)
  • Try to split into smaller PR to be merged in upstream.

sehz
sehz reviewed May 11, 2023
View reviewed changes
crates/fluvio-smartengine/src/engine/wasmedge/imp.rs Outdated
@@ -0,0 +1,114 @@
use super::instance::{WasmEdgeContext, WasmEdgeFn, WasmEdgeInstance};

use tracing::debug;
Copy link

Choose a reason for hiding this comment

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

can you reorder terms in of:

  • std
  • third party
  • fluvio dep

Copy link
Owner Author

Choose a reason for hiding this comment

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

That's OK. Are you going to review the PR now? I will resolve such issues if general ideas LGTY.

P.S. this PR is generally complete except minor points.

Copy link

Choose a reason for hiding this comment

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

Can you split this into PR just related to without WasmEdge? We need to discuss how to maintain WasmEdge related code.

Thanks

Copy link
Owner Author

Choose a reason for hiding this comment

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

That's OK. But I'd like to elaborate a bit: only folder wasmedge is WasmEdge-specific code, and that's ~300 lines of simple code. To discuss how to maintain WasmEdge related code, and to get the ideas of how the common code work, it might be better to keep WasmEdge related code now. This can actually make it easier to review this PR.

I can remove the WasmEdge related code after the code review if that's not decided.

Copy link

Choose a reason for hiding this comment

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

I would like have review by team but can't seem to add reviewers

Copy link
Owner Author

Choose a reason for hiding this comment

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

I created a PR to the main repo infinyon#3257

@xxchan xxchan force-pushed the xxchan/wasmedge branch from e8270bb to 8f7b787 Compare May 15, 2023 18:58
@xxchan xxchan force-pushed the xxchan/wasmedge branch from 8f7b787 to e6f7e2e Compare May 15, 2023 19:31
@xxchan xxchan mentioned this pull request May 31, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sehz sehz sehz left review comments

@juntao juntao juntao left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xxchan @juntao @sehz