abrown opened PR #2859 from runtime-wasi-nn
to main
:
This change makes the wasi-nn Cargo feature a default feature. Previously, a wasi-nn user would have to build a separate Wasmtime binary (e.g.
cargo build --features wasi-nn ...
) to use wasi-nn and the resulting binary would require OpenVINO shared libraries to be present in the environment in order to run (otherwise it would fail immediately with linking errors). With recent changes to theopenvino
crate, the wasi-nn implementation can defer the loading of the OpenVINO shared libraries until runtime (i.e., when the user Wasm program callswasi_ephemeral_nn::load
) and display a user-level error if anything goes wrong (e.g., the OpenVINO libraries are not present on the system). This runtime-linking addition allows the wasi-nn feature to be turned on by default and shipped with upcoming releases of Wasmtime. This change should be transparent for users who do not use wasi-nn: theopenvino
crate is small and the newly-available wasi-nn imports only affect programs in which they are used.For those interested in reviewing the runtime linking approach added to the
openvino
crate, see https://github.com/intel/openvino-rs/pull/19.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
abrown requested jedisct1 for a review on PR #2859.
abrown requested alexcrichton and jedisct1 for a review on PR #2859.
abrown requested alexcrichton, jedisct1 and sunfishcode for a review on PR #2859.
abrown requested tschneidereit, alexcrichton, jedisct1 and sunfishcode for a review on PR #2859.
abrown requested tschneidereit, alexcrichton, pchickey, jedisct1 and sunfishcode for a review on PR #2859.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
tschneidereit submitted PR Review.
tschneidereit submitted PR Review.
tschneidereit created PR Review Comment:
Hmm, I'm not sure how I feel about the drift between API and command line parameters here. I worry there might be some risk of someone learning the CLI parameters and then assuming
WasiModules::all()
means the same as--wasi-modules=all
.
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
I don't think this needs to be in the
wasmtime
crate since I don't think it's read by anything in thewasmtime
crate?
alexcrichton created PR Review Comment:
Instead of being an
assert
could this return a more first-class error to avoid panicking?
alexcrichton created PR Review Comment:
I think it's ok to avoid matching
-all-experimental
here since-all
suffices for this use case.
abrown submitted PR Review.
abrown created PR Review Comment:
Yup, moved to
src/lib.rs
.
abrown submitted PR Review.
abrown created PR Review Comment:
After discussing today, this now only matches
default
and-default
.
abrown submitted PR Review.
abrown created PR Review Comment:
WasiModules::all()
is now gone;"default"
corresponds toWasiModules::default()
and"-default"
corresponds toWasiModules::none()
.
abrown submitted PR Review.
abrown created PR Review Comment:
Ok, I went with
bail!
? I think some time down the road this type of check should be able to go away if we were to remove the special build flags for these WASI modules.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
abrown requested alexcrichton, pchickey, jedisct1 and sunfishcode for a review on PR #2859.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
abrown updated PR #2859 from runtime-wasi-nn
to main
.
alexcrichton submitted PR Review.
alexcrichton merged PR #2859.
Last updated: Dec 23 2024 at 12:05 UTC