Skip to content

perf(http/prom): avoid oneshot channel for request duration start time#4448

Open
unleashed wants to merge 1 commit intomainfrom
amr/record-start-time-optimization
Open

perf(http/prom): avoid oneshot channel for request duration start time#4448
unleashed wants to merge 1 commit intomainfrom
amr/record-start-time-optimization

Conversation

@unleashed
Copy link
Member

RecordRequestDuration knows the start time at call() time, whereas RecordResponseDuration defers it until the request body flushes. The latter needs async delivery from the request body wrapper, but the former is known synchronously.

Replace the shared oneshot::Receiver in ResponseState with an enum supporting both immediate and deferred start times, removing one heap allocation per request.

RecordRequestDuration knows the start time at call() time, whereas
RecordResponseDuration defers it until the request body flushes. The
latter needs async delivery from the request body wrapper, but the
former is known synchronously.

Replace the shared oneshot::Receiver<Instant> in ResponseState with an
enum supporting both immediate and deferred start times, removing one
heap allocation per request.

Signed-off-by: Alejandro Martinez Ruiz <amr@buoyant.io>
@unleashed unleashed requested a review from cratelyn March 6, 2026 15:59
@unleashed unleashed requested a review from a team as a code owner March 6, 2026 15:59
let state = self.labeler.mk_stream_labeler(&req).map(|labeler| {
let (tx, start) = oneshot::channel();
tx.send(time::Instant::now()).unwrap();
let start = super::StartTime::Known(Some(time::Instant::now()));
Copy link
Member

Choose a reason for hiding this comment

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

🏃💨

this is an astute observation. thank you for opening this, @unleashed!

pub(crate) enum StartTime {
/// Start time is already known.
Known(Option<time::Instant>),
/// Start time will be sent when the request body finishes streaming
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Start time will be sent when the request body finishes streaming
/// Start time will be sent when the request body finishes streaming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants