Skip to content
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.

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

Make ListBody<T> lazily deserializable #2310

Open
vilgotf opened this issue Jan 26, 2024 · 4 comments
Open

Make ListBody<T> lazily deserializable #2310

vilgotf opened this issue Jan 26, 2024 · 4 comments
Labels
c-http Affects the http crate t-feature Addition of a new feature

Comments

@vilgotf
Copy link
Member

vilgotf commented Jan 26, 2024

Instead of Response::models being an async fn -> Result<Vec<T>, E> it should return a type implementing Stream<Item = Result<T, E>> and IntoFuture<Output = Result<Vec<T>, E>>. This allows for lazy deserializing the response and potentially skip the (often unused) Vec buffer.

Inspiration: https://github.com/abdolence/axum-streams-rs

@vilgotf vilgotf added t-feature Addition of a new feature c-http Affects the http crate labels Jan 26, 2024
@Erk-
Copy link
Member

Erk- commented Jan 27, 2024

How would we even do lazy deserialzation? Or do you simply mean to defer the deserialzation to when it is actually looked at?

@laralove143
Copy link
Member

its possible in theory but i doubt serde supports it and im not sure the complexity this brings is worth the performance gain, in some cases it may even lead to worse performance when people repeatedly deserialize the stream's items

@vilgotf
Copy link
Member Author

vilgotf commented Feb 7, 2024

How would we even do lazy deserialzation? Or do you simply mean to defer the deserialzation to when it is actually looked at?

Hyper streams the response body by default so we just need two streaming adapters: &[u8] -> &str and &str -> T. For inspiration, tokio-websockets contains a streaming UTF-8 validator.

its possible in theory but i doubt serde supports it

Serde does not support streaming adapters but it's trivial to do so ourselves. We deserialize upon hitting a comma if our counter of braces and brackets are zero (i.e. the comma separates elements in the top level array).

and im not sure the complexity this brings is worth the performance gain, in some cases it may even lead to worse performance when people repeatedly deserialize the stream's items

The stream (AsyncIterator) would consume the response body

@Erk-
Copy link
Member

Erk- commented Feb 8, 2024

Would this not cause potentially more memory to be used since the string has to valid for as long as the deserilizer is?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-http Affects the http crate t-feature Addition of a new feature
Projects
None yet
Development

No branches or pull requests

3 participants