Skip to content

Commit 3b42424

Browse files
opt(s2n-quic-dc): Use RwLock for id/peer maps (#2799)
This uses the parking_lot RwLock which helps ensure fairness and guarantees that writers make forward progress even with a continuous stream of readers. In microbenchmarks, this essentially eliminates futex syscalls on the lock atomic, since readers are able to proceed in parallel. We likely still have cache line contention but that's cheaper than going to sleep on a Mutex and is a larger change to fix. The alignment to 128 bytes should also help with the contention.
1 parent 41c5aab commit 3b42424

File tree

1 file changed

+18
-22
lines changed
  • dc/s2n-quic-dc/src/path/secret/map

1 file changed

+18
-22
lines changed

dc/s2n-quic-dc/src/path/secret/map/state.rs

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,19 @@ use std::{
2828
mod tests;
2929

3030
#[derive(Default)]
31+
#[repr(align(128))]
3132
pub(crate) struct PeerMap(
32-
Mutex<hashbrown::HashTable<Arc<Entry>>>,
33+
parking_lot::RwLock<hashbrown::HashTable<Arc<Entry>>>,
3334
std::collections::hash_map::RandomState,
3435
);
3536

3637
#[derive(Default)]
37-
pub(crate) struct IdMap(Mutex<hashbrown::HashTable<Arc<Entry>>>);
38+
#[repr(align(128))]
39+
pub(crate) struct IdMap(parking_lot::RwLock<hashbrown::HashTable<Arc<Entry>>>);
3840

3941
impl PeerMap {
4042
fn reserve(&self, additional: usize) {
41-
self.0
42-
.lock()
43-
.unwrap_or_else(|e| e.into_inner())
44-
.reserve(additional, |e| self.hash(e));
43+
self.0.write().reserve(additional, |e| self.hash(e));
4544
}
4645

4746
fn hash(&self, entry: &Entry) -> u64 {
@@ -54,7 +53,7 @@ impl PeerMap {
5453

5554
pub(crate) fn insert(&self, entry: Arc<Entry>) -> Option<Arc<Entry>> {
5655
let hash = self.hash(&entry);
57-
let mut map = self.0.lock().unwrap_or_else(|e| e.into_inner());
56+
let mut map = self.0.write();
5857
match map.entry(hash, |other| other.peer() == entry.peer(), |e| self.hash(e)) {
5958
hashbrown::hash_table::Entry::Occupied(mut o) => {
6059
Some(std::mem::replace(o.get_mut(), entry))
@@ -68,29 +67,29 @@ impl PeerMap {
6867

6968
pub(crate) fn contains_key(&self, ip: &SocketAddr) -> bool {
7069
let hash = self.hash_key(ip);
71-
let map = self.0.lock().unwrap_or_else(|e| e.into_inner());
70+
let map = self.0.read();
7271
map.find(hash, |o| o.peer() == ip).is_some()
7372
}
7473

7574
pub(crate) fn get(&self, peer: SocketAddr) -> Option<Arc<Entry>> {
7675
let hash = self.hash_key(&peer);
77-
let map = self.0.lock().unwrap_or_else(|e| e.into_inner());
76+
let map = self.0.read();
7877
map.find(hash, |o| *o.peer() == peer).cloned()
7978
}
8079

8180
pub(crate) fn clear(&self) {
82-
let mut map = self.0.lock().unwrap_or_else(|e| e.into_inner());
81+
let mut map = self.0.write();
8382
map.clear();
8483
}
8584

8685
pub(super) fn len(&self) -> usize {
87-
let map = self.0.lock().unwrap_or_else(|e| e.into_inner());
86+
let map = self.0.read();
8887
map.len()
8988
}
9089

9190
fn remove_exact(&self, entry: &Arc<Entry>) -> Option<Arc<Entry>> {
9291
let hash = self.hash(entry);
93-
let mut map = self.0.lock().unwrap_or_else(|e| e.into_inner());
92+
let mut map = self.0.write();
9493
// Note that we are passing `eq` by **ID** not by address: this ensures that we find the
9594
// specific entry. The hash is still of the SocketAddr so we will look at the right entries
9695
// while doing this.
@@ -103,10 +102,7 @@ impl PeerMap {
103102

104103
impl IdMap {
105104
fn reserve(&self, additional: usize) {
106-
self.0
107-
.lock()
108-
.unwrap_or_else(|e| e.into_inner())
109-
.reserve(additional, |e| self.hash(e));
105+
self.0.write().reserve(additional, |e| self.hash(e));
110106
}
111107

112108
fn hash(&self, entry: &Entry) -> u64 {
@@ -119,7 +115,7 @@ impl IdMap {
119115

120116
pub(crate) fn insert(&self, entry: Arc<Entry>) -> Option<Arc<Entry>> {
121117
let hash = self.hash(&entry);
122-
let mut map = self.0.lock().unwrap_or_else(|e| e.into_inner());
118+
let mut map = self.0.write();
123119
match map.entry(hash, |other| other.id() == entry.id(), |e| self.hash(e)) {
124120
hashbrown::hash_table::Entry::Occupied(mut o) => {
125121
Some(std::mem::replace(o.get_mut(), entry))
@@ -134,29 +130,29 @@ impl IdMap {
134130
#[cfg(test)]
135131
pub(crate) fn contains_key(&self, id: &Id) -> bool {
136132
let hash = self.hash_key(id);
137-
let map = self.0.lock().unwrap_or_else(|e| e.into_inner());
133+
let map = self.0.read();
138134
map.find(hash, |o| o.id() == id).is_some()
139135
}
140136

141137
pub(crate) fn get(&self, id: Id) -> Option<Arc<Entry>> {
142138
let hash = self.hash_key(&id);
143-
let map = self.0.lock().unwrap_or_else(|e| e.into_inner());
139+
let map = self.0.read();
144140
map.find(hash, |o| *o.id() == id).cloned()
145141
}
146142

147143
pub(crate) fn clear(&self) {
148-
let mut map = self.0.lock().unwrap_or_else(|e| e.into_inner());
144+
let mut map = self.0.write();
149145
map.clear();
150146
}
151147

152148
pub(super) fn len(&self) -> usize {
153-
let map = self.0.lock().unwrap_or_else(|e| e.into_inner());
149+
let map = self.0.read();
154150
map.len()
155151
}
156152

157153
pub(super) fn remove(&self, id: Id) -> Option<Arc<Entry>> {
158154
let hash = self.hash_key(&id);
159-
let mut map = self.0.lock().unwrap_or_else(|e| e.into_inner());
155+
let mut map = self.0.write();
160156
match map.find_entry(hash, |other| *other.id() == id) {
161157
Ok(o) => Some(o.remove().0),
162158
Err(_) => None,

0 commit comments

Comments
 (0)