Support in-memory I/O APIs #493

jeffcharles · 2025-08-08T20:30:44Z

Support providers that use in-memory buffers instead of WASI hostcalls for I/O.

otheriault · 2025-09-18T14:54:16Z

src/engine.rs

+    // Need to track with deterministic order so don't use a hash
+    let mut imports = vec![];
+    for import in module.imports().map(|i| i.module().to_string()) {
+        if !imports.contains(&import) {


Does Rust not have an ordered set data structure we could collect to instead?

There isn't one in the standard library that preserves insertion order. There is a BTreeSet that will iterate over the keys in sorted order. FWIW, the valid cases are 0, 1, or 2 distinct modules that are being importing from, so the contains operation will be faster than the equivalent for any kind of set. Even in the invalid case, I'm not sure if the number of distinct modules would realistically go over a point where a set would offer a performance profile.

otheriault · 2025-09-18T15:13:10Z

src/io.rs

+                let log_len2 = u32::from_le_bytes(buf[20..24].try_into().unwrap()) as usize;
+
+                let mut output = vec![0; output_len];
+                memory.read(store.as_context(), output_offset, &mut output)?;


The memory advances an internal offset after every read I presume?

Nope. Conceptually you can think of the memory as a Vec<u8> in the store. The read operation just says start reading from this offset into the mutable buffer passed to it and takes the length of the mutable buffer as how much to read. There's no internal offset.

otheriault

I've never worked in this repo, but the changes and tests make sense to me. Maybe we should wait for another reviewer to approve.

adampetro · 2025-09-18T15:47:57Z

src/io.rs

+        let provider_path = format!("{module_name}.wasm");
+        let imported_module_bytes = StandardProviders::get(&provider_path);
+
+        if let Some(bytes) = imported_module_bytes {


When would imported_module_bytes be None?

If the Function module only imports from wasi_snapshot_preview1 and has no other imports.

adampetro · 2025-09-18T15:51:02Z

src/io.rs

+            if is_mem_io_provider(module_name) {
+                mem_io_instance = Some(imported_module_instance);
+            }


Should we error in the case that we hit this condition more than once?

Yes. But ideally I'd want that state to be unrepresentable since it's invalid. I'm thinking I might need to revisit the validation logic I added and maybe create a ValidatedModule wrapper or something like that.

I created a ValidatedModule abstraction to operate on. That said, it does rely on instantiation failing with unmet imports to handle some cases. Realistically, it's pretty hard to accidentally have multiple standard imports and code with multiple standard imports wouldn't work properly even if there were all linked. So I'm not too concerned about making that a nice experience given how unlikely it is to happen in practice.

adampetro · 2025-09-18T15:55:55Z

src/io.rs

+                logs.append(&mut logs2);
+
+                if logs.len() > FUNCTION_LOG_LIMIT {
+                    logs.splice(0..1, b"[TRUNCATED]...".iter().copied());


Am I understanding correctly that we aren't truncating anything here, just adding the [TRUNCATED]... at the start?

function-runner doesn't truncate but the provider does and the capacity for the provider's log limit is 1 byte larger than our log limit so we can detect that truncation happened.

adampetro · 2025-09-18T16:00:56Z

src/io.rs

+                .ok_or_else(|| anyhow!("Missing memory export named memory"))?
+                .write(store.as_context_mut(), input_offset as _, &self.input.raw)?;
+        }
+        Ok(())


Did we decide whether fuel counts during initialization? If not, do we need to set the fuel to u64::MAX again?

We decided it doesn't and we set it here.

That said, I can move the fuel setting logic to this method if it would be easier. I generally prefer to avoid performing unrelated side-effects though.

If we want to avoid side-effects we could store the fuel value at the start and re-set it at the end of this method like we proposed for finalize

adampetro · 2025-09-18T16:57:26Z

src/io.rs

+            IOStrategy::Memory(instance) => {
+                let instance = instance.expect("Should have been defined in initialize");
+                store.set_epoch_deadline(1); // Make sure we don't timeout while finalizing.
+                store.set_fuel(u64::MAX)?; // Make sure we don't run out of fuel finalizing.


I know that we check the fuel before calling this but I think it would be an easy enough mistake to move it after. Do we have a test catching that, or should we add some logic here to get the fuel at the start of this method and re-set it at the end of this method?

It'd probably be easier to reset it if we want to be defensive.

adampetro · 2025-09-18T20:00:53Z

src/validated_module.rs

+        let mut std_import = None;
+        for import in imports {
+            if let Some(bytes) = StandardProviders::get(&format!("{import}.wasm")) {
+                std_import = Some(Provider {
+                    bytes: bytes.data,
+                    name: import,
+                });
+                break;
+            }
+        }


would Iterator::find_map work here?

jeffcharles force-pushed the jc.in-memory-io branch from 9ffbf5a to 9d797ac Compare August 8, 2025 20:37

jeffcharles mentioned this pull request Aug 8, 2025

Refactor to remove scoping #494

Merged

jeffcharles force-pushed the jc.in-memory-io branch 7 times, most recently from 98dee32 to 2ea94e3 Compare August 18, 2025 15:29

jeffcharles force-pushed the jc.in-memory-io branch from 2ea94e3 to fa3bd87 Compare August 18, 2025 17:43

jeffcharles force-pushed the jc.in-memory-io branch 2 times, most recently from fcaa3bb to 0b10f1c Compare September 11, 2025 17:55

Support providers with in-memory I/O buffers

993e658

jeffcharles force-pushed the jc.in-memory-io branch from 0b10f1c to 993e658 Compare September 12, 2025 18:52

jeffcharles marked this pull request as ready for review September 17, 2025 14:44

jeffcharles requested a review from otheriault September 17, 2025 20:11

otheriault requested a review from adampetro September 18, 2025 14:44

otheriault reviewed Sep 18, 2025

View reviewed changes

adampetro reviewed Sep 18, 2025

View reviewed changes

jeffcharles added 3 commits September 18, 2025 15:24

Add ValidatedModule abstraction

5892d4f

Reset fuel consumed after finalization

9964f7b

Reset fuel in initialize

d9e1b95

adampetro approved these changes Sep 18, 2025

View reviewed changes

Use iterator methods

a664567

jeffcharles merged commit f719fe2 into main Sep 18, 2025
10 checks passed

jeffcharles deleted the jc.in-memory-io branch September 18, 2025 21:53

Support in-memory I/O APIs #493

Support in-memory I/O APIs #493

Uh oh!

Conversation

jeffcharles commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

otheriault left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeffcharles commented Aug 8, 2025 •

edited

Loading