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

Cranelift optimizations aren't enabled by default for the API. #981

Closed
peterhuene opened this issue Feb 25, 2020 · 2 comments · Fixed by #988
Closed

Cranelift optimizations aren't enabled by default for the API. #981

peterhuene opened this issue Feb 25, 2020 · 2 comments · Fixed by #988

Comments

@peterhuene
Copy link
Member

peterhuene commented Feb 25, 2020

We've recently turned optimizations on by default in the CLI.

I think we should probably enable optimizations by default for both the Rust and C APIs.

I propose we enable the cranelift optimization level to match the CLI for Engine::default() (thus Config::new sets the optimization level).

@alexcrichton
Copy link
Member

Agreed!

I don't think we necessarily want the CLI to match everything about the API, for example I think it's probably the way we want it to have the CLI have an on-by-default cache but the API has an off-by-default cache. For something like this though I think it's best to match the two.

@peterhuene
Copy link
Member Author

peterhuene commented Feb 25, 2020

I don't think we necessarily want the CLI to match everything about the API,

Totally. Off by default cache makes sense to me since the use cases for embedding are so varied. With respect to optimization, I would imagine that embedders will want optimized codegen almost all of the time and so it makes sense to have it on by default for such an important performance feature.

peterhuene added a commit to peterhuene/wasmtime that referenced this issue Feb 26, 2020
This commit changes the default opt-level for a new `Config` to `speed`.

Fixes bytecodealliance#981.
peterhuene added a commit to peterhuene/wasmtime that referenced this issue Feb 26, 2020
This commit changes the default opt-level for a new `Config` to `speed`.

Fixes bytecodealliance#981.
marmistrz pushed a commit to marmistrz/wasmtime that referenced this issue Mar 1, 2020
This commit changes the default opt-level for a new `Config` to `speed`.

Fixes bytecodealliance#981.
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 a pull request may close this issue.

2 participants