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

Enable multiple backends supported by the api crate to be used in the c-api as well #5443

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

xdoardo
Copy link
Collaborator

@xdoardo xdoardo commented Feb 28, 2025

(fixes #5224)

This PR adds to the wasm_engine_t enum the kinds V8, WAMR, WASMI and JSC to reflect the different possible kinds of engine that can actually be used in the underlying implementation. These enum variants can be by users used as a means to set the wanted engine in the underlying implementation.
Together with this addition, this PR changes the wasm_config_t type to (internally) hold an engine-specific configuration:

pub struct wasm_config_t {
    pub(super) engine: wasmer_engine_t,
    pub(super) engine_config: wasmer_engine_config_t, <------
    ...
}

where wasmer_engine_config_t is an enumeration of all the configurations for all the available engines:

enum wasmer_engine_config_t {
    #[cfg(feature = "sys")]
    Sys(sys::wasmer_sys_engine_config_t),

    #[cfg(feature = "jsc")]
    Jsc(jsc::wasmer_jsc_engine_config_t),
    
    ...
}

The two separate fields are needed because one, wasmer_engine_t, is public and users can set it; the other is definitely private and users cannot interact directly with it. This PR also introduces the relevant features to enable each of the backends cited above, with the matching X-default features to chose one as default.

Note: this PR produces a number of breaking changes in the API. Most notably, functions that were implicitly tied to the sys engine only, are now scoped with the sys name: wasm_config_canonicalize_nans is now wasm_config_sys_canonicalize_nans. Same goes for wasm_config_set_sys_target (previously wasm_config_set_target), wasm_config_sys_push_middleware (previously wasm_config_push_middleware), and wasm_config_set_sys_compiler (previously wasm_config_set_compiler).

Furthermore, a chage in the semantics of these functions is (debatably) introduced: what does it mean to call wasm_config_sys_push_middleware on a config whose selected wasmer_engine_t is not sys? In this PR the introduced behaviour is that of regardlessly change the selected engine to sys:

pub extern "C" fn wasm_config_set_sys_compiler(
    config: &mut wasm_config_t,
    compiler: wasmer_compiler_t,
) {
    ... 
    config.engine = wasmer_engine_t::UNIVERSAL;
    ... 
}

On one hand, this behaviour is transparent to the user - which should nonetheless expect the underlying engine to be the sys one, while on the other this function does not return a value, so we can't signal the user that the an error occurred in the case the current config.engine != sys.

Notice, also, that to maintain a bit of backwards compatibility, the enum constructor in the wasmer_engine_t corresponding to sys is still UNIVERSAL.

Missing bits:

  • More tests with all the backends enabled
  • CI jobs to target those tests

@xdoardo xdoardo requested a review from syrusakbary as a code owner February 28, 2025 13:54
Copy link

promptless bot commented Feb 28, 2025

📝 Documentation updates detected! A separate PR for documentation updates has been made here: wasmerio/docs.wasmer.io#118


impl Default for BackendKind {
fn default() -> Self {
#[cfg(feature = "sys-default")]
Copy link
Member

Choose a reason for hiding this comment

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

Can we make the features collide and show a compiler error if that's the case? (eg: you can't enable sys-default and wamr-default at the same time)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we already have this in the wasmer crate: https://github.com/wasmerio/wasmer/blob/main/lib/api/src/lib.rs#L457

#[cfg(feature = "sys")]
pub mod sys;
#[cfg(feature = "sys")]
pub use sys::wasm_config_set_sys_compiler;
Copy link
Member

Choose a reason for hiding this comment

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

I'd call this wasm_config_sys_set_compiler, so we can customize more settings in the future via wasm_config_sys_*

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pub struct wasmer_wasmi_engine_config_t;

/// Create a new [`wasm_engine_t`] backed by a `wasmi` engine.
pub fn wasm_wasmi_engine_new_with_config(config: wasm_config_t) -> Option<Box<wasm_engine_t>> {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pub struct wasmer_wamr_engine_config_t;

/// Create a new [`wasm_engine_t`] backed by a `wamr` engine.
pub fn wasm_wamr_engine_new_with_config(config: wasm_config_t) -> Option<Box<wasm_engine_t>> {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pub struct wasmer_v8_engine_config_t;

/// Create a new [`wasm_engine_t`] backed by a `v8` engine.
pub fn wasm_v8_engine_new_with_config(config: wasm_config_t) -> Option<Box<wasm_engine_t>> {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}

/// Create a new [`wasm_engine_t`] backed by a `sys` engine.
pub fn wasm_sys_engine_new_with_config(config: wasm_config_t) -> Option<Box<wasm_engine_t>> {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pub struct wasmer_jsc_engine_config_t;

/// Create a new [`wasm_engine_t`] backed by a `jsc` engine.
pub fn wasm_jsc_engine_new_with_config(config: wasm_config_t) -> Option<Box<wasm_engine_t>> {
Copy link
Member

Choose a reason for hiding this comment

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

This should not be public

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Enable c_api-backends to be used in Wasmer's implementation of the wasm_c_api
2 participants