Stream: git-wasmtime

Topic: wasmtime / issue #10278 Usefulness of UserExternalNameRef

Wasmtime GitHub notifications bot (Feb 23 2025 at 13:57):

awillenbuecher-xq-tec opened issue #10278:

Feature

It seems to me that the UserExternalNameRef ⟶ UserExternalName indirection in Function/FunctionParameters is mostly useless. I think I understand the motivation behind it, which is to replace referenced functions and data in copies of the function stencil. But that could just as well be done when resolving the relocations – which you'll have to do anyway after you change any UserExternalNameRef ⟶ UserExternalName associations. I might be misunderstanding its purpose though, so please correct me if I'm wrong.

The downside to this indirection is that it forces _every_ user to go through the UserExternalNameRef, whether it benefits them or not. This is not a showstopper, but a bit of an annoyance.

Benefit

A simpler interface that is both easier to understand and more convenient to use. No more need to keep track of and lookup the UserExternalNameRef ⟶ UserExternalName mapping. This would simplify code within Cranelift itself as well as code using Cranelift.

Implementation

Change the ExternalName enum in extname.rs to

pub enum ExternalName {
    User(UserExternalName),
    ...

remove user_named_funcs and user_ext_name_to_ref from FunctionParameters, and replace any uses and lookups of UserExternalNameRef directly with UserExternalName.

I'd be happy to prepare a PR.

Alternatives

If there are use cases which require keeping track of which UserExternalNames are referenced in a Function, then a referenced_names: HashSet<UserExternalName> could be added to FunctionParameters, but without requiring users to register names if they don't need to keep track of this information.

Wasmtime GitHub notifications bot (Feb 24 2025 at 06:04):

cfallin closed issue #10278:

Feature

It seems to me that the UserExternalNameRef ⟶ UserExternalName indirection in Function/FunctionParameters is mostly useless. I think I understand the motivation behind it, which is to replace referenced functions and data in copies of the function stencil. But that could just as well be done when resolving the relocations – which you'll have to do anyway after you change any UserExternalNameRef ⟶ UserExternalName associations. I might be misunderstanding its purpose though, so please correct me if I'm wrong.

The downside to this indirection is that it forces _every_ user to go through the UserExternalNameRef, whether it benefits them or not. This is not a showstopper, but a bit of an annoyance.

Benefit

A simpler interface that is both easier to understand and more convenient to use. No more need to keep track of and lookup the UserExternalNameRef ⟶ UserExternalName mapping. This would simplify code within Cranelift itself as well as code using Cranelift.

Implementation

Change the ExternalName enum in extname.rs to

pub enum ExternalName {
    User(UserExternalName),
    ...

remove user_named_funcs and user_ext_name_to_ref from FunctionParameters, and replace any uses and lookups of UserExternalNameRef directly with UserExternalName.

I'd be happy to prepare a PR.

Alternatives

If there are use cases which require keeping track of which UserExternalNames are referenced in a Function, then a referenced_names: HashSet<UserExternalName> could be added to FunctionParameters, but without requiring users to register names if they don't need to keep track of this information.

Wasmtime GitHub notifications bot (Feb 24 2025 at 06:04):

cfallin commented on issue #10278:

The reason for this indirection is that it improves reuse (hit rate) in the incremental compilation cache. For pragmatic reasons, the Wasmtime embedding of Cranelift makes a direct mapping of UserExternalName to Wasm function indices. One of the goals of the incremental compilation design was to allow for some "index slippage" as a Wasm module is changed during development and still be able to reuse most of the compilations. We make this work by adding the level of indirection: so for example, if Wasm function 1000 calls functions 1001 and 1002, and later the user adds a function and rebuilds the module so we have function 1001 calling 1002 and 1003 (but the bodies are all the same otherwise), we would have different UserExternalNames but by referring to UserExternalNameRef indices 0 and 1 instead (and substituting 0->1001, 1->1002 or 0->1002, 1->1003 in the cached compilation result) we can reuse the same compilation.

Because of this, we can't remove UserExternalNameRef. Hopefully that clears things up!

Last updated: Feb 28 2025 at 03:10 UTC