Skip to content

Make streams owned by request/response that they are tied to. #6228

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.

Sign up for GitHub

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

Merged
pchickey merged 6 commits into from
Apr 21, 2023
Merged

Make streams owned by request/response that they are tied to. #6228

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c7b3f25
Make streams owned by request/response that they are tied to.
brendandburns Apr 18, 2023
a72d8e0
Address comments, fix tests.
brendandburns Apr 19, 2023
14b25d2
Address comment.
brendandburns Apr 20, 2023
22a2dd5
Update crates/wasi-http/src/streams_impl.rs
brendandburns Apr 20, 2023
bf2c3ba
Switch to BytesMut
brendandburns Apr 20, 2023
b855a4b
Merge branch 'ptr' of https://github.com/brendandburns/wasmtime into ptr
brendandburns Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Switch to BytesMut
  • Loading branch information
@brendandburns
brendandburns committed Apr 20, 2023
commit bf2c3ba24d87f747dec793ace82b1aa4885463f1
3 changes: 2 additions & 1 deletion crates/wasi-http/src/http_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,8 @@ impl WasiHttp {
.get(&request.body)
.unwrap_or(&Stream::default())
.data
.clone(),
.clone()
.freeze(),
);
let t = timeout(first_bytes_timeout, sender.send_request(call.body(body)?)).await?;
let mut res = t?;
Expand Down
20 changes: 4 additions & 16 deletions crates/wasi-http/src/streams_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::poll::Pollable;
use crate::streams::{InputStream, OutputStream, StreamError};
use crate::WasiHttp;
use anyhow::{anyhow, bail};
use bytes::BufMut;
use std::vec::Vec;

impl crate::streams::Host for WasiHttp {
Expand Down Expand Up @@ -74,21 +73,10 @@ impl crate::streams::Host for WasiHttp {
buf: Vec<u8>,
) -> wasmtime::Result<Result<u64, StreamError>> {
let len = buf.len();
match self.streams.get(&this) {
Some(st) => {
if st.closed {
bail!("stream is dropped!");
}
let new_len = st.data.len() + len;
let mut new = bytes::BytesMut::with_capacity(new_len);
new.put(st.data.clone());
new.put(bytes::Bytes::from(buf));
self.streams.insert(this, new.freeze().into());
}
None => {
self.streams.insert(this, bytes::Bytes::from(buf).into());
}
}
self.streams.entry(this)
.or_default()
.data
.extend_from_slice(buf.as_slice());
Ok(Ok(len.try_into()?))
}

Expand Down
8 changes: 5 additions & 3 deletions crates/wasi-http/src/struct.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::types::{Method, Scheme};
use bytes::Bytes;
use bytes::{BufMut, Bytes, BytesMut};
use std::collections::HashMap;

#[derive(Clone, Default)]
pub struct Stream {
Copy link
Member

@rvolosatovs rvolosatovs Apr 19, 2023

Choose a reason for hiding this comment

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

Looking at the usages of this struct, it seems that a From<Bytes> implementation could really clear this up

Copy link
Contributor Author

@brendandburns brendandburns Apr 19, 2023

Choose a reason for hiding this comment

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

added.

pub closed: bool,
pub data: Bytes,
pub data: BytesMut,
}

#[derive(Clone)]
Expand Down Expand Up @@ -80,9 +80,11 @@ impl Stream {

impl From<Bytes> for Stream {
fn from(bytes: Bytes) -> Self {
let mut buf = BytesMut::with_capacity(bytes.len());
buf.put(bytes);
Self {
Copy link
Member

@rvolosatovs rvolosatovs Apr 19, 2023

Choose a reason for hiding this comment

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

If we had a Default trait impl, this could have just called Self::default()

Copy link
Contributor Author

@brendandburns brendandburns Apr 19, 2023

Choose a reason for hiding this comment

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

done.

closed: false,
data: bytes,
data: buf,
}
}
}
Expand Down