Skip to content

Commit

Permalink
fix(s2n-quic-dc): make debug assertions cheaper for TCP accept manager
Browse files Browse the repository at this point in the history
  • Loading branch information
camshaft committed Dec 12, 2024
1 parent 0006a28 commit f7e3e71
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 37 deletions.
30 changes: 13 additions & 17 deletions dc/s2n-quic-dc/src/stream/server/tokio/tcp/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ where
let capacity = workers.len();
let mut free = List::default();
for idx in 0..capacity {
free.push(&mut workers, idx);
free.push_back(&mut workers, idx);
}

let by_sojourn_time = List::default();
Expand Down Expand Up @@ -186,7 +186,7 @@ where

self.inner
.by_sojourn_time
.push(&mut self.inner.workers, idx);
.push_back(&mut self.inner.workers, idx);

// kick off the initial poll to register wakers with the socket
self.inner.poll_worker(idx, cx, publisher, clock);
Expand Down Expand Up @@ -282,7 +282,8 @@ where

// the worker is all done so indicate we have another free slot
self.by_sojourn_time.remove(&mut self.workers, idx);
self.free.push(&mut self.workers, idx);
// use `push_front` instead to avoid cache churn
self.free.push_front(&mut self.workers, idx);

cf
}
Expand All @@ -293,7 +294,7 @@ where
C: Clock,
{
// if we have a free worker then use that
if let Some(idx) = self.free.pop(&mut self.workers) {
if let Some(idx) = self.free.pop_front(&mut self.workers) {
trace!(op = %"next_worker", free = idx);
return Some(idx);
}
Expand All @@ -304,7 +305,7 @@ where
// if the worker's sojourn time exceeds the maximum, then reclaim it
if sojourn >= self.max_sojourn_time() {
trace!(op = %"next_worker", injected = idx, ?sojourn);
return self.by_sojourn_time.pop(&mut self.workers);
return self.by_sojourn_time.pop_front(&mut self.workers);
}

trace!(op = %"next_worker", ?sojourn, max_sojourn_time = ?self.max_sojourn_time());
Expand All @@ -317,18 +318,13 @@ where

#[cfg(debug_assertions)]
fn invariants(&self) {
for idx in 0..self.workers.len() {
assert!(
self.free
.iter(&self.workers)
.chain(self.by_sojourn_time.iter(&self.workers))
.filter(|v| *v == idx)
.count()
== 1,
"worker {idx} should be linked at all times\n{:?}",
self.workers[idx].link,
);
}
let mut linked_workers = self
.free
.iter(&self.workers)
.chain(self.by_sojourn_time.iter(&self.workers))
.collect::<Vec<_>>();
linked_workers.sort();
assert!((0..self.workers.len()).eq(linked_workers.iter().copied()));

let mut expected_free_len = 0usize;
for idx in self.free.iter(&self.workers) {
Expand Down
87 changes: 67 additions & 20 deletions dc/s2n-quic-dc/src/stream/server/tokio/tcp/manager/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl List {
}

#[inline]
pub fn pop<L>(&mut self, entries: &mut [L]) -> Option<usize>
pub fn pop_front<L>(&mut self, entries: &mut [L]) -> Option<usize>
where
L: AsMut<Link>,
{
Expand Down Expand Up @@ -80,7 +80,30 @@ impl List {
}

#[inline]
pub fn push<L>(&mut self, entries: &mut [L], idx: usize)
pub fn push_front<L>(&mut self, entries: &mut [L], idx: usize)
where
L: AsMut<Link>,
{
debug_assert!(idx < usize::MAX);

let head = self.head;
if head != usize::MAX {
entries[head].as_mut().prev = idx;
} else {
debug_assert!(self.is_empty());
self.tail = idx;
}
self.head = idx;

let link = entries[idx].as_mut();
link.prev = usize::MAX;
link.next = head;

self.set_linked_status(idx, true);
}

#[inline]
pub fn push_back<L>(&mut self, entries: &mut [L], idx: usize)
where
L: AsMut<Link>,
{
Expand Down Expand Up @@ -237,15 +260,31 @@ mod tests {
impl CheckedList {
#[inline]
fn pop(&mut self, entries: &mut [Link]) -> Option<usize> {
let v = self.list.pop(entries);
let v = self.list.pop_front(entries);
assert_eq!(v, self.oracle.pop_front());
self.invariants(entries);
v
}

#[inline]
fn push(&mut self, entries: &mut [Link], v: usize) {
self.list.push(entries, v);
fn push(&mut self, entries: &mut [Link], v: usize, front: bool) {
if front {
self.push_front(entries, v);
} else {
self.push_back(entries, v);
}
}

#[inline]
fn push_front(&mut self, entries: &mut [Link], v: usize) {
self.list.push_front(entries, v);
self.oracle.push_front(v);
self.invariants(entries);
}

#[inline]
fn push_back(&mut self, entries: &mut [Link], v: usize) {
self.list.push_back(entries, v);
self.oracle.push_back(v);
self.invariants(entries);
}
Expand Down Expand Up @@ -279,7 +318,7 @@ mod tests {
let locations = (0..LEN).map(|_| Location::A).collect();

for idx in 0..LEN {
a.push(&mut entries, idx);
a.push_back(&mut entries, idx);
}

Self {
Expand All @@ -293,55 +332,63 @@ mod tests {

impl Harness {
#[inline]
fn transfer(&mut self, idx: usize) {
fn transfer(&mut self, idx: usize, front: bool) {
let location = &mut self.locations[idx];
match location {
Location::A => {
self.a.remove(&mut self.entries, idx);
self.b.push(&mut self.entries, idx);
self.b.push(&mut self.entries, idx, front);
*location = Location::B;
}
Location::B => {
self.b.remove(&mut self.entries, idx);
self.a.push(&mut self.entries, idx);
self.a.push(&mut self.entries, idx, front);
*location = Location::A;
}
}
}

#[inline]
fn pop_a(&mut self) {
fn pop_a(&mut self, front: bool) {
if let Some(v) = self.a.pop(&mut self.entries) {
self.b.push(&mut self.entries, v);
self.b.push(&mut self.entries, v, front);
self.locations[v] = Location::B;
}
}

#[inline]
fn pop_b(&mut self) {
fn pop_b(&mut self, front: bool) {
if let Some(v) = self.b.pop(&mut self.entries) {
self.a.push(&mut self.entries, v);
self.a.push(&mut self.entries, v, front);
self.locations[v] = Location::A;
}
}
}

#[derive(Clone, Copy, Debug, TypeGenerator)]
enum Op {
Transfer(#[generator(0..LEN)] usize),
PopA,
PopB,
Transfer {
#[generator(0..LEN)]
idx: usize,
front: bool,
},
PopA {
front: bool,
},
PopB {
front: bool,
},
}

#[test]
fn invariants_test() {
check!().with_type::<Vec<Op>>().for_each(|ops| {
let mut harness = Harness::default();
for op in ops {
match op {
Op::Transfer(idx) => harness.transfer(*idx),
Op::PopA => harness.pop_a(),
Op::PopB => harness.pop_b(),
match *op {
Op::Transfer { idx, front } => harness.transfer(idx, front),
Op::PopA { front } => harness.pop_a(front),
Op::PopB { front } => harness.pop_b(front),
}
}
})
Expand Down

0 comments on commit f7e3e71

Please sign in to comment.