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

Use resources #59

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Use resources #59

merged 1 commit into from
Nov 9, 2023

Conversation

abrown
Copy link
Collaborator

@abrown abrown commented Oct 28, 2023

Now that component model resources are specified and implemented, it should be possible to use the resource type for specifying tensors, graphs and execution contexts.

Closes #47.

Now that component model `resource`s are specified and implemented, it
should be possible to use the `resource` type for specifying tensors,
graphs and execution contexts.

Closes WebAssembly#47.
@abrown abrown marked this pull request as ready for review November 9, 2023 20:44
@abrown
Copy link
Collaborator Author

abrown commented Nov 9, 2023

Like #61, this will be merged as we discussed in the 10-31 meeting.

@abrown abrown merged commit ac355f3 into WebAssembly:main Nov 9, 2023
1 check passed
@abrown abrown deleted the resources branch November 9, 2023 22:49
abrown added a commit to bytecodealliance/wasmtime that referenced this pull request Jun 25, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline
abrown added a commit to bytecodealliance/wasmtime that referenced this pull request Jun 25, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full
abrown added a commit to abrown/wasmtime that referenced this pull request Jun 25, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (bytecodealliance#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full
abrown added a commit to abrown/wasmtime that referenced this pull request Jun 26, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (bytecodealliance#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full
abrown added a commit to abrown/wasmtime that referenced this pull request Jun 26, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (bytecodealliance#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full
abrown added a commit to abrown/wasmtime that referenced this pull request Jun 26, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (bytecodealliance#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full
abrown added a commit to abrown/wasmtime that referenced this pull request Jun 26, 2024
Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (bytecodealliance#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full
github-merge-queue bot pushed a commit to bytecodealliance/wasmtime that referenced this pull request Jun 27, 2024
* wasi-nn: use resources

Recent discussion in the wasi-nn proposal (see [wasi-nn#59], e.g.) has
concluded that the right approach for representing wasi-nn "things"
(tensors, graph, etc.) is with a component model _resource_. This
sweeping change brings Wasmtime's implementation in line with that
decision.

Initially I had structured this PR to remove all of the WITX-based
implementation (#8530). But, after consulting in a Zulip [thread] on
what other WASI proposals aim to do, this PR pivoted to support _both_`
the WITX-based and WIT-based ABIs (e.g., preview1 era versus preview2,
component model era). What is clear is that the WITX-based specification
will remain "frozen in time" while the WIT-based implementation moves
forward.

What that means for this PR is a "split world" paradigm. In many places,
we have to distinguish between the `wit` and `witx` versions of the same
thing. This change isn't the end state yet: it's a big step forward
towards bringing Wasmtime back in line with the WIT spec but, despite my
best efforts, doesn't fully fix all the TODOs left behind over several
years of development. I have, however, taken the liberty to refactor and
fix various parts as I came across them (e.g., the ONNX backend). I plan
to continue working on this in future PRs to figure out a good error
paradigm (the current one is too wordy) and device residence.

[wasi-nn#59]: WebAssembly/wasi-nn#59
[thread]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/wasi-nn's.20preview1.20vs.20preview2.20timeline

prtest:full

* vet: audit `ort`-related crate updates

* Simplify `WasiNnView`

With @alexcrichton's help, this change removes the `trait WasiNnView`
and `struct WasiNnImpl` wrapping that the WIT-based implementation used
for accessing the host context. Instead, `WasiNnView` is now a `struct`
containing the mutable references it needs to make things work. This
unwraps one complex layer of abstraction, though it does have the
downside that it complicates CLI code to split borrows of `Host`.

* Temporarily disable WIT check

* Refactor errors to use `trappable_error_type`

This change simplifies the return types of the host implementations of
the WIT-based wasi-nn. There is more work to be done with errors, e.g.,
to catch up with the upstream decision to return errors as resources.
But this is better than the previous mess.
abrown added a commit to abrown/wasi-nn-spec that referenced this pull request Aug 13, 2024
As discussed in WebAssembly#43, there is no requirement to set up tensors prior to
calling `compute` as well as retrieving them separately afterwards. As
of WebAssembly#59, passing around tensors is cheap (they're resources now), so
there is no data copy necessary if we adopt this PR. This change
proposes removing the `set-input` and `get-output` functions, moving all
of the tensor-passing to `compute`. Closes WebAssembly#43.
abrown added a commit that referenced this pull request Oct 28, 2024
As discussed in #43, there is no requirement to set up tensors prior to
calling `compute` as well as retrieving them separately afterwards. As
of #59, passing around tensors is cheap (they're resources now), so
there is no data copy necessary if we adopt this PR. This change
proposes removing the `set-input` and `get-output` functions, moving all
of the tensor-passing to `compute`. Closes #43.
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.

Use component model resources
1 participant