transport: unify request modifiers and reduce allocations #2396
Conversation
Update the hello-world tutorial readme to reflect changes to package names and remove reference to deprecated tools. ## Motivation As someone relatively new to Rust and new to Tonic, I was working through the hello-world tutorial and found a few issues. I've successfully built the project with the changes in this PR. ## Solution - update example Cargo.toml to reflect package name updates - update example build.rs with correct build tool name - remove reference to deprecated GUI tool I've tested this and it runs and builds correctly. I sort of hate to reference Postman as the recommended GUI but it was the first one on the list referenced by the now-deprecated Bloom GUI.
Fixes: hyperium#2386 Interop tests are failing since the [protobuf codegen requires the same version as protoc](https://github.com/protocolbuffers/protobuf/blob/2ae8154f366a6d776bcc3ac931413bc4d99f578e/rust/release_crates/protobuf_codegen/src/lib.rs#L136-L148) to work. This PR pins the protoc version to fix the tests.
|
While I feel like this is a valiant effort, I am not convinced about the improvement to performance here. I think a big change like this would require a lot of data to prove that its worth the complexity. The reason these things are implemented in their own service (at least originally) was to provide a clear cut difference in what they do, trying to abstract over them imo is a bit over engineered without a proven benefit. |
|
Totally fair — thanks for the feedback. I don’t want to push an abstraction that feels over-engineered at this stage. |
Motivation
Currently, request modifiers (
AddOrigin,UserAgent) are implemented as separate services, each carrying their ownimpl Service. This leads to duplicated boilerplate, scattered logic, and some unnecessary runtime allocations (e.g. cloningUri, building headers at call boundaries).Additionally, the derivation of the endpoint’s origin was handled speculatively in
connection.rsrather than directly onEndpoint, making reasoning about the canonical origin less clear.The goal of this change is to:
Endpoint.Solution
AddOriginandUserAgentintorequest_modifiers.rs.Modifier<M, T>that centralizes theService<Request>impl.into_fn(), avoiding repeatedServiceimpls.Endpoint::get_origin()to move origin derivation closer to the data it represents.UserAgentonce at construction, instead of at every service call.Uriclones by borrowing where possible.try_insertstill respected for user-provided headers).✅ No public API changes.
✅ Cleaner internals, less duplication.
✅ Small but meaningful perf improvements (fewer heap allocations/clones).