Skip to content

Commit 143849d

Browse files
authored
feat: cache effective retransmission priority in SendStream (#3592)
* feat: cache effective retransmission priority in `SendStream` `write_stream_frame` recomputed `priority + retransmission_priority` — a two-level enum match — on every call, including priority-level passes that don't match and return immediately. Surprisingly, this showed up in server profiles. Replace the write-only `retransmission_priority` field with `effective_priority`, computed once in `set_priority`. * Fixes * Add tests
1 parent f0469ce commit 143849d

1 file changed

Lines changed: 76 additions & 6 deletions

File tree

neqo-transport/src/send_stream.rs

Lines changed: 76 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -685,7 +685,8 @@ pub struct SendStream {
685685
state: State,
686686
conn_events: ConnectionEvents,
687687
priority: TransmissionPriority,
688-
retransmission_priority: RetransmissionPriority,
688+
/// Cached result of `priority + retransmission`, recomputed in `set_priority`.
689+
effective_priority: TransmissionPriority,
689690
retransmission_offset: u64,
690691
sendorder: Option<SendOrder>,
691692
bytes_sent: u64,
@@ -708,7 +709,7 @@ impl SendStream {
708709
},
709710
conn_events,
710711
priority: TransmissionPriority::default(),
711-
retransmission_priority: RetransmissionPriority::default(),
712+
effective_priority: TransmissionPriority::default() + RetransmissionPriority::default(),
712713
retransmission_offset: 0,
713714
sendorder: None,
714715
bytes_sent: 0,
@@ -751,13 +752,13 @@ impl SendStream {
751752
self.fair
752753
}
753754

754-
pub const fn set_priority(
755+
pub fn set_priority(
755756
&mut self,
756757
transmission: TransmissionPriority,
757758
retransmission: RetransmissionPriority,
758759
) {
759760
self.priority = transmission;
760-
self.retransmission_priority = retransmission;
761+
self.effective_priority = transmission + retransmission;
761762
}
762763

763764
#[must_use]
@@ -909,7 +910,7 @@ impl SendStream {
909910
) {
910911
let retransmission = if priority == self.priority {
911912
false
912-
} else if priority == self.priority + self.retransmission_priority {
913+
} else if priority == self.effective_priority {
913914
true
914915
} else {
915916
return;
@@ -994,7 +995,7 @@ impl SendStream {
994995
State::ResetSent {
995996
ref mut priority, ..
996997
} => {
997-
*priority = Some(self.priority + self.retransmission_priority);
998+
*priority = Some(self.effective_priority);
998999
}
9991000
State::ResetRecvd { .. } => (),
10001001
_ => unreachable!(),
@@ -3240,4 +3241,73 @@ mod tests {
32403241
s.mark_as_acked(len_u64, 0, true);
32413242
assert!(s.is_terminal());
32423243
}
3244+
3245+
fn stream_with_priority(tx: TransmissionPriority, rx: RetransmissionPriority) -> SendStream {
3246+
let mut s = SendStream::new(
3247+
StreamId::from(0),
3248+
100,
3249+
connection_fc(100),
3250+
ConnectionEvents::default(),
3251+
);
3252+
s.set_priority(tx, rx);
3253+
s
3254+
}
3255+
3256+
fn stream_frames_written(s: &mut SendStream, priority: TransmissionPriority) -> usize {
3257+
let mut builder =
3258+
packet::Builder::short(Encoder::default(), false, None::<&[u8]>, packet::LIMIT);
3259+
let mut tokens = recovery::Tokens::new();
3260+
let mut stats = FrameStats::default();
3261+
s.write_stream_frame(priority, &mut builder, &mut tokens, &mut stats);
3262+
stats.stream
3263+
}
3264+
3265+
fn reset_frame_written(s: &mut SendStream, priority: TransmissionPriority) -> bool {
3266+
let mut builder =
3267+
packet::Builder::short(Encoder::default(), false, None::<&[u8]>, packet::LIMIT);
3268+
let mut tokens = recovery::Tokens::new();
3269+
let mut stats = FrameStats::default();
3270+
s.write_reset_frame(priority, &mut builder, &mut tokens, &mut stats)
3271+
}
3272+
3273+
#[test]
3274+
fn set_priority_updates_effective_priority() {
3275+
let mut s = stream_with_priority(
3276+
TransmissionPriority::Low,
3277+
RetransmissionPriority::MuchHigher,
3278+
);
3279+
s.send(&[0x42; 10]).unwrap();
3280+
3281+
assert_eq!(stream_frames_written(&mut s, TransmissionPriority::Low), 1);
3282+
s.mark_as_lost(0, 10, false);
3283+
assert_eq!(
3284+
stream_frames_written(&mut s, TransmissionPriority::Normal),
3285+
0
3286+
);
3287+
assert_eq!(
3288+
stream_frames_written(
3289+
&mut s,
3290+
TransmissionPriority::Low + RetransmissionPriority::MuchHigher,
3291+
),
3292+
1,
3293+
);
3294+
}
3295+
3296+
#[test]
3297+
fn reset_lost_uses_effective_priority() {
3298+
let mut s = stream_with_priority(
3299+
TransmissionPriority::Normal,
3300+
RetransmissionPriority::MuchHigher,
3301+
);
3302+
s.send(b"hello").unwrap();
3303+
s.reset(0);
3304+
3305+
assert!(reset_frame_written(&mut s, TransmissionPriority::Normal));
3306+
s.reset_lost();
3307+
assert!(!reset_frame_written(&mut s, TransmissionPriority::Normal));
3308+
assert!(reset_frame_written(
3309+
&mut s,
3310+
TransmissionPriority::Normal + RetransmissionPriority::MuchHigher,
3311+
));
3312+
}
32433313
}

0 commit comments

Comments
 (0)