ureq: ureq::Error is massive

[src/main.rs:17] std::mem::size_of::<ureq::Error>() = 576

It would be nice if this wasn’t so large so that users don’t need to Box it to avoid copying around a lot of unused memory in cases like Result<(), ureq::Error>::Ok(()).

Unfortunately, the size seems to mostly come from Response, and it’s publicly visible that ureq::Error::Http contains a Response-by-value, so I don’t know any way to non-breakingly fix this other than boxing Response’s data internally, which would affect performance on the success path too.

[src/main.rs:18] std::mem::size_of::<ureq::Response>() = 432
[src/main.rs:19] std::mem::size_of::<ureq::Transport>() = 568

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 1
Comments: 15 (13 by maintainers)

Most upvoted comments

Rather than boxing Response.unit, we should get rid of it. It’s only used in one place, Response::into_reader, to determine if the request was a HEAD. That, in turn, is used to build a LimitedRead of 0. When we implement https://github.com/algesten/ureq/issues/264, a Response to a HEAD request will have a zero-length Vec body, and so Response::into_reader won’t need to know it was a HEAD.

Boxing rustls::StreamOwned: I think that’s definitely reasonable, particularly given that it’s private to the implementation. It also makes sense conceptually - the stream’s lifetime is longer than any Response that uses it, and ownership of the stream is handed back and forth between ConnectionPool and Response. However, I’m wary of making performance-motivated changes without a benchmark. As discussed in this thread, we could wind up harming performance instead of helping it; we could wind up performance neutral; or we could wind up improving performance, only for it to backslide later without us noticing.

I think we should make this change if (a) we write a benchmark showing it’s an improvement, or (b) if we find documentation or a blog post specific to the Rust compiler saying, e.g. “for values that are passed on the stack, it’s generally worthwhile to box anything above X bytes.”

jsha on Jan 9, 2021