@Alex Crichton and I were just discussing where to land the implementation of wasi-nn: in the Wasmtime repo temporarily or in its own separate repository? My impression from our last Wasmtime meeting was that, even though it wasn't optimal, @Dan Gohman agreed with me that placing it in the Wasmtime repository temporarily was ok.
My reasons went something like: having multiple Wasmtime binaries floating around, one for each combination of WASI modules a user may want, is confusing for users and a pain for those of us who would have to maintain it. Until we have a plugin system, stuffing wasi-nn into Wasmtime as an optional feature and adding CI testing to keep it working would prevent it from getting out of date as Wasmtime changes (e.g. it depends on wiggle, @Pat Hickey, and having it in-tree would mean that we could immediately see if changes in wiggle break wasi-nn)
Yeah, I think my earlier instinct was to look for ways to have it in a separate repo, but I can see the logic in having it in the main repo too
I personally feel that this is best done by moving it into a separate repository until the wasi-nn proposal has progressed further. My thinking is that if we try to prototype all wasi proposals in-tree it could make the tree pretty unwieldy (things like slow CI time, maybe large clones, lots of flags/configuration to wade through, etc)
I would prefer to land in-progress proposals out-of-tree and tackle the problems that brings up (like @Andrew Brown is mentioning)
e.g. we should library-ify the CLI interface, make it super easy to plug in your own wasi proposal into the CLI, focus on stabilizing interfaces like witx, etc.
I have a different intuition on this, fwiw: I think there'd be a lot of value in making even experimental features available for easy testing in Wasmtime, meaning we build them by default, so they can be used with runtime flags. They wouldn't be runtime-enabled by default, of course, but there's a huge accessibility cliff between features that're included in the default build, and those that aren't
however, I very much appreciate that there are meaningful tradeoffs to this, so I wonder if we should have a more in-depth conversation on the policies around this more generally, not just for wasi-nn
We could go both ways on minimizing costs really, so perhaps we could game out both scenarios for a bit and evaluate based on the end state?
e.g. if adding it to the repo is incredibly cheap then it's fine, but if the cheapest we can get it is still pretty pricy that may swing the other way
yeah, that's a good point. Though I'd be surprised if find that it'd be very pricey to add something to the repo, but at the same time it's somehow very easy to use the feature from outside the repo. And no matter what, there's just a huge difference between "you can just use the feature in the default build" and "you have to build things yourself
one of my main worries about adding it in-repo is the CI cost
each proposal would be a new builder (presumably) with presumably only one maintainer to ping when things go wrong
for something as valuable as CI that's a small bus factor for halting things
I think we would also want it such that if you don't enable the feature the clone/build cost is as close as possible to what it is today
each proposal would be a new builder
can you say more about this part? ISTM if the feature is enabled by default, it'd not require an extra builder?
with presumably only one maintainer to ping when things go wrong
this is an excellent point, and something I very much agree we should avoid. So perhaps to be enabled by default, a proposal has to have a commitment by a stakeholder group — i.e. a team — to actively maintain it and address issues in a timely manner
I think we would also want it such that if you don't enable the feature the clone/build cost is as close as possible to what it is today
I think my PR to merge wasi-nn in-tree meets that criteria: if the Cargo feature isn't enabled, the Wasmtime build time would not be impacted
@Till Schneidereit, not sure if enabling wasi-nn by default is currently desirable (though I agree with making things super-easy to run): enabling it requires you to have OpenVINO installed on the system (which is... unlikely) or tries to build OpenVINO from source (which takes forever)
I think eventually we need some form of "plugin" system to add capabilities to Wasmtime closer to runtime (along the lines of what you meant, @Till Schneidereit, but perhaps not all the way to "runtime flags"). Having wasi-nn in-tree would motivate the creation of such a system, if only to get wasi-nn out of tree.
@Andrew Brown ah yeah, that does sound like enabling it by default wouldn't be desirable right now. It's not possible to dynamically link to OpenVINO by any chance, thereby turning it into a runtime dependency?
Well, OpenVINO does rely on shared libraries... maybe I'm being too careful, but I assumed that we wouldn't want to build the openvino
crate unless the OpenVINO shared libraries are present on the system (either through binary installation or by building from source). Are you proposing we could just depend on the openvino
crate and then the user will see a dynamic linking failure when they try to use it if they do not have the right libraries on their system?
@Till Schneidereit oh I'm assuming wasi-nn is off-by-default but we still want to test wherever it lives
so we'll need a CI builder running all the wasi-nn tests, and if that breaks it blocks everyone else from landing code until it's fixed
My general thinking is that experimental wasi proposals shouldn't cause a burden on other developers, and the ways I can envision a burden are: CI time and flakiness, build time, and clone size
Not to say the current PR doesn't handle build time and/or clone size, I think it does, just want to list things out!
My general thinking is that experimental wasi proposals shouldn't cause a burden on other developers, and the ways I can envision a burden are: CI time and flakiness, build time, and clone size
I agree... and we should list the inverse side as well: experimental wasi proposals shouldn't be a burden for the developers who write them :laughing:
so we'll need a CI builder running all the wasi-nn tests, and if that breaks it blocks everyone else from landing code until it's fixed
That is partly good, though? If wiggle changes enough to break wasi-nn in CI, at least we can see that and fix it, right?
I'm less worried about things like wiggle refactorings causing breakage, where the wiggle refactor would ideally take into account the impact on consumers where if they're in-repo they can be evaluated
(although that can also be problematic because it may be hard to build an extension like wasi-nn);
I'm more worried about wasi-nn CI breaking
like it's just flaky for some wasi-nn-specific reason, like OpenVINO downloads go offline or something like that
or OpenVINO takes forever to build on CI (things like that, these are just examples)
so part of my thinking here is informed by how JS runtimes do make experimental features available with very low overhead, and how that's really useful for the ecosystem. I.e., you can enable most experimental JS features and tons of in-progress bundled modules and APIs in Node.js by just using runtime flags, instead of having to do custom builds. That means it becomes really easy to play around with them and validate their usefulness to content authors.
We might well simply not be there in terms of maturity of WASI, the ecosystem, or Wasmtime, but I do think it's something worth striving for
I mean, I had not considered the runtime flag idea much before, but I now think it would be possible if we are ok exposing the user to "library not found" errors (though we could provide some helpful error messages there); that approach would then imply a CI step like the one Alex doesn't want to depend on, though. I do agree that the wasi-nn CI step I added is just one more thing that can go wrong
(even though I would commit to fixing it if it breaks)
sorry to be clear I'm just trying to be clear about the costs of putting it in-repo
I don't mean to pass judgement on whether it's worth it or not
I agree with you and Till that in-repo is the best for development of the proposal itself
entirely understood, and I'm glad you're raising these concerns. We should absolutely not enable experimental features lightly and without feeling confident that the results won't be painful
I agree with you and Till that in-repo is the best for development of the proposal itself
@Alex Crichton, so how should we conclude this conversation? Do you want to discuss more?
I think we need more input, my point about CI is something I don't think we can avoid and I don't want to speak for everyone else
if we want to move forward with this I think there needs to be more buy-in and consensus about what to do about CI
To give some input / my own perspective on the CI issue -- I do find that it is somewhat productivity-draining to wait for the long tail of CI builds, and I'd be worried about anything involving a large build of a dependency. Is there a way we could cache it, or download a prebuilt image/release of OpenVINO, or...?
As a general point: would it make sense to think about "tiers" in the CI at some point? E.g. in the SpiderMonkey/mozilla world, we had tests that would be run nightly but wouldn't block a checkin. (IIRC that Rust does something similar?)
(Perhaps that's a separate/tangential conversation, but seems to overlap at least here w.r.t. the notion of add-on / experimental pieces)
The wasi-nn CI step I added in the PR takes ~5 minutes--this is significantly shorter than other steps, like the x64 backend (~11 minutes), building the API documentation (~18 minutes), test (~17 minutes), or build (~30 minutes). That is mainly because it uses a prebuilt OpenVINO binary that it installs as a GitHub action.
@Chris Fallin, I like the tiered idea: it would be nice to see that certain things fail (e.g. peephole optimizer stuff, @fitzgen (he/him)) but they shouldn't absolutely block merging.
Last updated: Dec 23 2024 at 13:07 UTC