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

fix(c-api): give a cleaner error message if cargo isn't found #8497

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

dundargoc
Copy link
Contributor

@dundargoc dundargoc commented Apr 27, 2024

Instead of

Performing build step for
'wasmtime-crate''WASMTIME_CARGO_BINARY-NOTFOUND' is not recognized as an
internal or external command, operable program or batch file.

this will now instead output

"cargo" was not found. Ensure "cargo" is in PATH. Aborting...

@dundargoc dundargoc requested a review from a team as a code owner April 27, 2024 13:41
@dundargoc dundargoc requested review from fitzgen and removed request for a team April 27, 2024 13:41
@github-actions github-actions bot added the wasmtime:c-api Issues pertaining to the C API. label Apr 27, 2024
crates/c-api/CMakeLists.txt Outdated Show resolved Hide resolved
Instead of

> Performing build step for
'wasmtime-crate''WASMTIME_CARGO_BINARY-NOTFOUND' is not recognized as an
internal or external command, operable program or batch file.

this will now instead output

> "cargo" was not found. Ensure "cargo" is in PATH. Aborting...
@fitzgen fitzgen added this pull request to the merge queue Apr 29, 2024
Merged via the queue into bytecodealliance:main with commit 1fa6d4f Apr 29, 2024
23 checks passed
@dundargoc dundargoc deleted the fix/error_message branch April 29, 2024 15:39
dundargoc added a commit to dundargoc/wasmtime that referenced this pull request May 7, 2024
…dealliance#8497)

Instead of

> Performing build step for
'wasmtime-crate''WASMTIME_CARGO_BINARY-NOTFOUND' is not recognized as an
internal or external command, operable program or batch file.

this will now instead output

> "cargo" was not found. Ensure "cargo" is in PATH. Aborting...
alexcrichton pushed a commit that referenced this pull request May 7, 2024
* fix(c-api): give a cleaner error message if cargo isn't found (#8497)

Instead of

> Performing build step for
'wasmtime-crate''WASMTIME_CARGO_BINARY-NOTFOUND' is not recognized as an
internal or external command, operable program or batch file.

this will now instead output

> "cargo" was not found. Ensure "cargo" is in PATH. Aborting...

* c-api: use `--release` when MinSizeRel and RelWithDebInfo is used (#8549)

* build: add "fastest-runtime" profile for runtime optimization (#8554)

This is extremely useful for cases where the default optimizations just
are not enough.

Background: [neovim](https://github.com/neovim/neovim) is interested to
add wasmtime support in neovim/neovim#28415 but
we noticed that including wasmtime, even when not using wasmtime
directly, heavily affects runtime performance. This is not only
reflected in the increased startuptime but affects the runtime
performance overall.

Here are the benchmarks for startuptimes for different configurations.
Important to note is that all of runtime is affected, but the
startuptime is a decent proxy to measure runtime performance:

```
No wasm
  Time (mean ± σ):      50.5 ms ±   1.5 ms    [User: 32.8 ms, System: 12.3 ms]
  Range (min … max):    48.3 ms …  54.4 ms    56 runs

Wasm, lto=thin
  Time (mean ± σ):     104.9 ms ±   3.5 ms    [User: 86.5 ms, System: 12.7 ms]
  Range (min … max):    99.5 ms … 111.1 ms    26 runs

Wasm, lto=true
  Time (mean ± σ):      83.8 ms ±   2.5 ms    [User: 65.8 ms, System: 12.1 ms]
  Range (min … max):    80.5 ms …  93.3 ms    31 runs

Wasm, lto=true, strip="none", incremental=false, codegen-units=1, panic="abort"
  Time (mean ± σ):      53.1 ms ±   1.0 ms    [User: 35.5 ms, System: 12.5 ms]
  Range (min … max):    50.6 ms …  55.5 ms    54 runs
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants