search

containerd / runwasi

Facilitates running Wasm / WASI workloads managed by containerd
Apache License 2.0
1.01k stars 82 forks source link

refactor(*): refactor wasm bytes preparation logic to the core crate #448

Closed Mossaka closed 5 months ago

Mossaka commented 5 months ago

This PR refactors the common Source logic from runtime shim to the core crate. To do so, the core Engine trait API has a breaking change:

  1. run_wasi() now takes a wasm_bytes: &[u8] parameter as the wasm module / component loaded from the file system or OCI layers to execute.

cc @jsturtevant

jprendes commented 5 months ago

I think I would prefer if Source provided a method called to_bytes, that would encapsulate this logic, rather than changing the run_wasi signature.

jsturtevant commented 5 months ago

The background is that we had something similiar to to_bytes but this had a clone https://github.com/containerd/runwasi/pull/401#discussion_r1414601347. Maybe I've got it wrong in that thread about trying to avoid a clone on that call?

I am also not a fan of these changes to the run_wasi trait

jprendes commented 5 months ago

The clone could be avoided returning a Cow<[u8]> IIUC.

jprendes commented 5 months ago

Alternatively, in the case of File the bytes could be cached to avoid reading the file multiple times (something like a OnceCell or OnceLock), and then the &[u8] would refer to the bytes owned by the cache.

Mossaka commented 5 months ago

@jprendes thanks for the suggestion of Cow! I re-implement the refactor in https://github.com/containerd/runwasi/pull/454

Mossaka commented 5 months ago

Close in favor of #454