Skip to content

feat: add PluginBuilder::with_wasmtime_config#764

Merged
zshipko merged 3 commits intomainfrom
expose-config
Sep 19, 2024
Merged

feat: add PluginBuilder::with_wasmtime_config#764
zshipko merged 3 commits intomainfrom
expose-config

Conversation

@zshipko
Copy link
Copy Markdown
Contributor

@zshipko zshipko commented Sep 18, 2024

An alternative to #763, this PR allows an initial wasmtime::Config to be passed in when building a plugin. Some of these values may be overwritten by the Extism runtime, but it allows for things like static memory size and other low-level details to be handled directly instead of us having to wrap every option ourselves.

@nilslice
Copy link
Copy Markdown
Member

Feels like this is safe to do, since we plan to consider dropping Wasmtime for most SDKs except possibly an async-friendly Rust SDK, right?

@zshipko
Copy link
Copy Markdown
Contributor Author

zshipko commented Sep 18, 2024

Yeah, that's the goal, and we can always replace this with some other configuration options when we drop wasmtime, since there will be some refactoring needed at that point anyway - until then this gives users of the current Rust SDK some control over the low-level stuff that we don't necessarily need to care about.

@zshipko zshipko merged commit 7bf41c2 into main Sep 19, 2024
@zshipko zshipko deleted the expose-config branch September 19, 2024 18:27
@SebastianHambura
Copy link
Copy Markdown
Contributor

Some of these values may be overwritten by the Extism runtime

Sorry if I'm a bit late, but I think it would be quite usefull to know which values are overwritten. I can already imagine a user changing settings and wondering why nothing is changing. From what I could quickly see, here's the spot where we overwrite some settings:

        let mut config = config.unwrap_or_default();
        config
            .async_support(false)
            .epoch_interruption(true)
            .debug_info(debug_options.debug_info)
            .coredump_on_trap(debug_options.coredump.is_some())
            .profiler(debug_options.profiling_strategy)
            .wasm_tail_call(true)
            .wasm_function_references(true)
            .wasm_gc(true);

https://github.com/extism/extism/blob/25bcbe092d94562441a2eb54a74584e023cc1e05/runtime/src/plugin.rs#L334C1-L343C28

Could we change with_wasmtime_config documentation to something like:


Configure an initial wasmtime config to be passed to the plugin

Warning: some values might be may be overwritten by the Extism runtime. In particular:

  • async_support
  • epoch_interruption
  • debug_info
  • coredump_on_trap
  • profiler
  • wasm_tail_call
  • wasm_function_references
  • wasm_gc

See the implementation details of [this function](Plugin::build_new) to check which values are overwritten.

@zshipko
Copy link
Copy Markdown
Contributor Author

zshipko commented Sep 23, 2024

Good idea, can you open a PR with this change?

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.

3 participants