Skip to content

Commit ca64719

Browse files
committed
Refactor merging of additional TSN blocks
The logic for adding new TSNs to the list of received blocks was complex and difficult to follow. It involved a binary search followed by multiple conditional checks to determine how to merge the new TSN with existing blocks. This commit refactors the `add_additional_tsn` method to be more readable and efficient. It now finds the correct insertion point for the new TSN and then uses a simple match statement to handle merging with the previous block, the next, both, or neither.
1 parent 3a45b84 commit ca64719

File tree

1 file changed

+25
-38
lines changed

1 file changed

+25
-38
lines changed

src/rx/data_tracker.rs

Lines changed: 25 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use crate::packet::sack_chunk::SackChunk;
2020
use crate::timer::Timer;
2121
use crate::types::Tsn;
2222
use std::cmp::min;
23-
use std::cmp::Ordering;
2423
use std::ops::Range;
2524
use std::time::Duration;
2625
use std::time::Instant;
@@ -126,50 +125,38 @@ impl DataTracker {
126125
}
127126

128127
fn add_additional_tsn(&mut self, tsn: Tsn) -> bool {
129-
let idx = self
130-
.additional_tsn_blocks
131-
.binary_search_by(|s| if s.end < tsn { Ordering::Less } else { Ordering::Greater })
132-
.unwrap_err();
133-
134-
match self.additional_tsn_blocks.get(idx) {
135-
None => {
136-
// No matching block found. There is no greater than, or equal block - which means
137-
// that this TSN is greater than any block. It can then be inserted at the end.
138-
self.additional_tsn_blocks.push(tsn..tsn + 1);
139-
true
140-
}
141-
Some(range) if range.contains(&tsn) => {
142-
// It's already in this block.
143-
false
128+
let idx = self.additional_tsn_blocks.partition_point(|r| r.start <= tsn);
129+
130+
// Check if it's a duplicate in the block before insertion point.
131+
if idx > 0 && self.additional_tsn_blocks[idx - 1].contains(&tsn) {
132+
return false;
133+
}
134+
135+
let extend_prev = idx > 0 && self.additional_tsn_blocks[idx - 1].end == tsn;
136+
let extend_next = idx < self.additional_tsn_blocks.len()
137+
&& self.additional_tsn_blocks[idx].start == tsn + 1;
138+
139+
match (extend_prev, extend_next) {
140+
(true, true) => {
141+
// Merge with previous and next block.
142+
let next_end = self.additional_tsn_blocks[idx].end;
143+
self.additional_tsn_blocks[idx - 1].end = next_end;
144+
self.additional_tsn_blocks.remove(idx);
144145
}
145-
Some(range) if range.end == tsn => {
146-
// This block can be expanded to the right, or merged with the next.
147-
if idx + 1 < self.additional_tsn_blocks.len()
148-
&& self.additional_tsn_blocks[idx + 1].start == tsn + 1
149-
{
150-
// Expanding it would make it adjacent to next block - merge those.
151-
self.additional_tsn_blocks[idx].end = self.additional_tsn_blocks[idx + 1].end;
152-
self.additional_tsn_blocks.remove(idx + 1);
153-
} else {
154-
// Expand to the right.
155-
self.additional_tsn_blocks[idx].end = tsn + 1;
156-
}
157-
true
146+
(true, false) => {
147+
// Extend previous block.
148+
self.additional_tsn_blocks[idx - 1].end = tsn + 1;
158149
}
159-
Some(range) if tsn + 1 == range.start => {
160-
// This block can be expanded to the left. Merging to the left would've been covered
161-
// by the above "merge to the right". Both blocks (expand a right-most block to the
162-
// left and expand a left-most block to the right) would match, but the left-most
163-
// would be returned by std::lower_bound.
150+
(false, true) => {
151+
// Extend next block.
164152
self.additional_tsn_blocks[idx].start = tsn;
165-
true
166153
}
167-
Some(_) => {
168-
// Need to create a new block in the middle.
154+
(false, false) => {
155+
// Insert new block.
169156
self.additional_tsn_blocks.insert(idx, tsn..tsn + 1);
170-
true
171157
}
172158
}
159+
true
173160
}
174161

175162
/// Call for every incoming data chunk. Returns `true` if `tsn` was seen for the first time, and

0 commit comments

Comments
 (0)