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

feat: support WasmEdge as an alternative engine #1

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

Conversation

xxchan
Copy link
Owner

@xxchan xxchan commented Feb 26, 2023

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 force-pushed the xxchan/wasmedge branch 2 times, most recently from 95c20a2 to 9dfbb6b Compare April 15, 2023 12:49
@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.

@@ -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

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.

3 participants