Skip to content

Enable inlining by default#13214

Closed
alexcrichton wants to merge 1 commit intobytecodealliance:mainfrom
alexcrichton:inlining
Closed

Enable inlining by default#13214
alexcrichton wants to merge 1 commit intobytecodealliance:mainfrom
alexcrichton:inlining

Conversation

@alexcrichton
Copy link
Copy Markdown
Member

This commit switches the default configuration of Wasmtime to enable cross-module inlining by default. This means that components with multiple modules will, by default, inline small functions across these modules. Additionally modules which use the GC proposal will have inlining performed by default. Intra-module inlining without GC, however, will continue to not happen.

This required two minor updates in tests. One is to disable inlining when the exact call stack is expected and another is to disable inlining when the stack-switching proposal is enabled because it currently falls over in Cranelift on the stack_switch instruction.

This commit switches the default configuration of Wasmtime to enable
cross-module inlining by default. This means that components with
multiple modules will, by default, inline small functions across these
modules. Additionally modules which use the GC proposal will have
inlining performed by default. Intra-module inlining without GC,
however, will continue to not happen.

This required two minor updates in tests. One is to disable inlining
when the exact call stack is expected and another is to disable inlining
when the stack-switching proposal is enabled because it currently falls
over in Cranelift on the `stack_switch` instruction.
@alexcrichton
Copy link
Copy Markdown
Member Author

cc @fitzgen and @cfallin, this is initially a draft since I wanted to have somewhere to ask the question of "does anyone know a reason to not do this?" and figured that a minimal PR is good enough. So, to follow-through with that, do y'all know of a reason to not enable this by default? My main motivation would be optimizations like #13194 where for WASIp3 we'll want to have that on-by-default.

@cfallin
Copy link
Copy Markdown
Member

cfallin commented Apr 27, 2026

do y'all know of a reason to not enable this by default?

At an algorithmic level we'd want to make sure our heuristics are robust enough that we don't have blowups (in compilation time or in size of binary); but I guess we've been fuzzing this for a long time now and haven't seen any such issues. That would have been my only concern, otherwise 👍

@fitzgen
Copy link
Copy Markdown
Member

fitzgen commented Apr 27, 2026

Yeah I feel fine as far as our heuristics and fuzzing go. Ofc the heuristics need tweaks to be Good but that can happen at any time as separate work.

The one thing I would be tentative about here is the impact on compile times. I remember seeing some cases where they were quite a bit slower with inlining (although I never systematically investigated this) and I'd want to make sure that (a) compilation times for single core modules is not affected and (b) that we are okay with whatever slow down we actually see for compiling components.

@github-actions github-actions Bot added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime labels Apr 27, 2026
@github-actions
Copy link
Copy Markdown

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

  • If you added a new Config method, you wrote extensive documentation for
    it.

    Details

    Our documentation should be of the following form:

    Short, simple summary sentence.
    
    More details. These details can be multiple paragraphs. There should be
    information about not just the method, but its parameters and results as
    well.
    
    Is this method fallible? If so, when can it return an error?
    
    Can this method panic? If so, when does it panic?
    
    # Example
    
    Optional example here.
    
  • If you added a new Config method, or modified an existing one, you
    ensured that this configuration is exercised by the fuzz targets.

    Details

    For example, if you expose a new strategy for allocating the next instance
    slot inside the pooling allocator, you should ensure that at least one of our
    fuzz targets exercises that new strategy.

    Often, all that is required of you is to ensure that there is a knob for this
    configuration option in wasmtime_fuzzing::Config (or one
    of its nested structs).

    Rarely, this may require authoring a new fuzz target to specifically test this
    configuration. See our docs on fuzzing for more details.

  • If you are enabling a configuration option by default, make sure that it
    has been fuzzed for at least two weeks before turning it on by default.


Details

To modify this label's message, edit the .github/label-messager/wasmtime-config.md file.

To add new label messages or remove existing label messages, edit the
.github/label-messager.json configuration file.

Learn more.

@alexcrichton
Copy link
Copy Markdown
Member Author

Discussion at the Cranelift meeting today concluded:

  1. One avenue is that it should be confirmed that the quite-different inlining compilation path, when no inlining actually happens, should be double-checked to have little-to-no overehead.
  2. Another avenue is that we should strive to, by default, not tamper with wasm backtraces. While it's possible to use frame tables built for debugging to recover inlining decisions after-the-fact that's not currently implemented. Given that we should probably change the defaults to only allowing inlining of intrinsics, for example, and have a dedicated mode for "inline some wasm things, but beware backtraces will be weird"

@alexcrichton
Copy link
Copy Markdown
Member Author

I've morphed this PR into #13254 and #13250, so I'll close this in favor of those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants