-
-
Notifications
You must be signed in to change notification settings - Fork 354
Description
TLDR: this is a request to have http::request::Parts::new() and http::response::Parts::new() exposed as public constructors for the respective Parts.
I was profiling a (non-public) application which produces Request values in a hot loop through the usual http::request::Builder flow, and (after filtering out I/O-related activity) I was surprised to see that this builder shows up performing memory copying in a relatively large number of samples:

After looking a bit into it, I think that the current setup of the Builder forces the compiler to emit a lot of memcpy calls to move the data (inner Parts and the arguments) through the callstack. I guess this is related to the fallible inner part already reported at #330.
I tried to reproduce these observations on a minimal example like this:
fn main() {
let mut count = 0usize;
for _i in 0..100_000_000 {
let method = http::Method::from_bytes(b"GET").unwrap();
// Relevant part.
let req_builder = http::request::Builder::new().method(method);
let req = req_builder.body(()).unwrap();
//
if req.method() == &http::Method::GET {
count += 1;
}
}
println!("Count: {}", count);
}And I can see (in release mode) that most of the time is spent moving memory around:

As I have all the pieces to build a proper Parts, I'd like to avoid all of these steps and go through Request::from_parts() instead of the Builder. That is something like this:
let mut parts = http::request::Parts::new();
parts.method = method;
let req = http::request::Request::from_parts(parts, ());However I haven't see a way for external consumers to build a Parts, as the new() method above is currently private (and there is an internal _priv field).
Would it be ok to make the two existing Parts::new public?
For reference, I tried to expose that and use the snippet above, and there is a visible difference in profiles, both in my application and in the above reproducer:

Maybe not much relevant as a microbenchmark, but hyperfine also confirms the above difference between the two approaches: the run time of the two reproducers is 8.363s (± 0.490s) with the Builder and 4.497s (± 0.747s) with Parts::new().