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

update to Wasmtime 17 and WASI 0.2.0-rc-2023-12-05 #2222

Merged
merged 3 commits into from
Jan 11, 2024

Conversation

dicej
Copy link
Contributor

@dicej dicej commented Jan 11, 2024

As of this writing, Wasmtime 17 has not yet been released, so we're temporarily pointing at the release-17.0.0 branch. By the time it is released, WASI 0.2.0-rc-2023-12-05 will become WASI 0.2.0, at which point we'll need to update the WIT files and code to match.

The upshot is that we'll be supporting three snapshots indefinitely: 2023-10-18, 2023-11-10, and 2023-12-05 AKA the final 0.2.0 release.

Most of the changes here deal with supporting those three snapshots, while the rest deal with the Hyper 1.0 update, which led to a bit of awkwardness when mixing Hyper and Reqwest, since the latter has not yet been updated to use Hyper 1.0.

As of this writing, Wasmtime 17 has not yet been released, so we're temporarily
pointing at the `release-17.0.0` branch.  By the time it is released, WASI
0.2.0-rc-2023-12-05 will become WASI 0.2.0, at which point we'll need to update
the WIT files and code to match.

The upshot is that we'll be supporting three snapshots indefinitely: 2023-10-18,
2023-11-10, and 2023-12-05 AKA the final 0.2.0 release.

Most of the changes here deal with supporting those three snapshots, while the
rest deal with the Hyper 1.0 update, which led to a bit of awkwardness when
mixing Hyper and Reqwest, since the latter has not yet been updated to use Hyper
1.0.

Signed-off-by: Joel Dice <[email protected]>
Copy link
Collaborator

@rylev rylev left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@dicej
Copy link
Contributor Author

dicej commented Jan 11, 2024

I haven't been able to repro the CI test failures yet -- will keep investigating.

@dicej
Copy link
Contributor Author

dicej commented Jan 11, 2024

Wasn't able to repro the CI failures on Mac or Linux. Look like they're hitting a 20 second timeout waiting for spin up to start listening on the specified port, but they only take a couple of seconds to run on my machines. Maybe even our beefy 4-core runner is struggling today? Maybe we should consider running the integration tests using a release build of Spin instead of a debug build?

@dicej
Copy link
Contributor Author

dicej commented Jan 11, 2024

I just ran the same tests on main, and they're not any faster there (when I run them locally), so there doesn't seem to be any actual performance regression here.

@rylev
Copy link
Collaborator

rylev commented Jan 11, 2024

@dicej sigh - I ran into the same issue... I think unfortunately the right move is probably to increase the time out. We could run with a release build, but then we have to pay the price of building that release artificat which is way more time than any reasonable timeout. I believe the timeout is currently at 20 seconds. I say we raise it to 2 minutes. If we hit the timeout still then we might want to think about how to make our debug builds faster.

One additional idea I have is to run the built artifacts through wasm-tools strip. Smaller wasm binaries tend to load much faster. I'd like to be able to do that without requiring the user has wasm-tools on their path though.

We're not sure yet why we've started consistently exceeding the old 20 second
timeout on tests which run in a 2-3 seconds locally, but that's what's
happening.  If it turns out even 2 minutes is insufficient, we'll need to dig
deeper.

Signed-off-by: Joel Dice <[email protected]>
@dicej dicej enabled auto-merge (squash) January 11, 2024 18:01
@dicej dicej merged commit b7d6576 into fermyon:main Jan 11, 2024
11 checks passed
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.

4 participants