Skip to content

feat(bindgen): add support for stream reads with count > 1#1441

Open
andreiltd wants to merge 1 commit intobytecodealliance:mainfrom
andreiltd:add-byte-stream
Open

feat(bindgen): add support for stream reads with count > 1#1441
andreiltd wants to merge 1 commit intobytecodealliance:mainfrom
andreiltd:add-byte-stream

Conversation

@andreiltd
Copy link
Copy Markdown
Member

@andreiltd andreiltd commented May 4, 2026

This adds readMany(count) support to generated p3 streams so host-side byte consumers can perform Canonical ABI stream reads instead of pulling one byte at a time. It uses the transferred count from the packed stream result when reading host buffers.

I believe that the current implementation is correct, it's just that iteratrd on single bytes is very inefficient. I originally came across this when tried to run p3_cli_hello_stdout test from wasmtime:

    async fn run() -> Result<(), ()> {
        let (mut tx, rx) = wit_stream::new();
        futures::join!(
            async {
                wasi::cli::stdout::write_via_stream(rx).await.unwrap();
            },
            async {
                tx.write(b"hello, world\n".to_vec()).await;
                drop(tx);
            },
        );
        Ok(())
    }

The write function is a bit surprising in the sense that it will not loop until the data is sent over, but instead it will try to send the data at most once. In jco we will receive a single byte and report transferred = 1, which is correct behavior. The looping function is write_all which with our current implementation will indeed print the whole string.

I need to have a look if tx.write reports warning that we are dropping result that must be used, which would make the debugging much easier.

@andreiltd andreiltd requested a review from vados-cosmonic as a code owner May 4, 2026 13:37
@andreiltd andreiltd marked this pull request as draft May 5, 2026 13:41
@andreiltd andreiltd force-pushed the add-byte-stream branch 2 times, most recently from af586d5 to e89e776 Compare May 6, 2026 17:51
@vados-cosmonic
Copy link
Copy Markdown
Collaborator

Hey @andreiltd should I be reviewing this just yet? You just rebased I think but should I wait for the other PRs to land in main?

@andreiltd
Copy link
Copy Markdown
Member Author

andreiltd commented May 6, 2026

Hey @vados-cosmonic yeah, not yet, this depends (includes): #1445

but it looks like 1445 is failing so I will have to figure out that first and then I will rebase this one.

This adds `readMany(count)` support to generated p3 streams so host-side
byte consumers can perform Canonical ABI stream reads instead of pulling
one byte at a time. It uses the transferred count from the packed stream
result when reading host buffers.
@andreiltd andreiltd marked this pull request as ready for review May 7, 2026 09:29
@andreiltd
Copy link
Copy Markdown
Member Author

It's ready now.

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.

2 participants