Don't lock eviction parent set if it's likely empty

bench/benches/userland.rs

@@ -50,7 +50,7 @@ pub fn block<M: MeasurementInfo>(c: &mut Criterion<M>, do_parse: bool) {
                 .into_iter()
                 .filter_map(NonZeroU32::new)
                 .collect(),
-            p_expires: vec![0.0],
+            p_expires: vec![0.0, 0.1],
         }),
     ];
 

lib/opte/src/engine/flow_table.rs

@@ -25,6 +25,7 @@ use core::num::NonZeroU16;
 use core::num::NonZeroU32;
 use core::sync::atomic::AtomicBool;
 use core::sync::atomic::AtomicU64;
+use core::sync::atomic::AtomicUsize;
 use core::sync::atomic::Ordering;
 #[cfg(all(not(feature = "std"), not(test)))]
 use illumos_sys_hdrs::uintptr_t;
@@ -208,12 +209,16 @@ impl<S: FlowState> FlowEntryInfo for FlowEntry<S> {
         // If we have an explicit priority, we keep the most-protected (lowest)
         // priority which we know of.
         let mut best_prio = own_prio;
-        for maybe_child in &*self.lifetime.children.read() {
-            if let Some(child) = maybe_child.0.upgrade() {
-                match (best_prio, child.eviction_priority(now)) {
-                    (None, a) => best_prio = a,
-                    (Some(_), None) => {}
-                    (Some(old), Some(new)) => best_prio = Some(new.min(old)),
+        if self.lifetime.n_children.load(Ordering::Relaxed) > 0 {
+            for maybe_child in &*self.lifetime.children.read() {
+                if let Some(child) = maybe_child.0.upgrade() {
+                    match (best_prio, child.eviction_priority(now)) {
+                        (None, a) => best_prio = a,
+                        (Some(_), None) => {}
+                        (Some(old), Some(new)) => {
+                            best_prio = Some(new.min(old))
+                        }
+                    }
                 }
             }
         }
@@ -230,6 +235,7 @@ impl<S: FlowState> FlowEntryInfo for FlowEntry<S> {
             children.len() < Self::MAX_CHILDREN || children.contains(&to_place);
         if can_insert {
             children.insert(to_place);
+            self.lifetime.n_children.store(children.len(), Ordering::Relaxed);
             Ok(())
         } else {
             Err(OpteError::MaxCapacity(
@@ -241,6 +247,7 @@ impl<S: FlowState> FlowEntryInfo for FlowEntry<S> {
     fn remove_child(&self, child: &Arc<dyn FlowEntryInfo>) {
         let mut children = self.lifetime.children.write();
         children.remove(&ByAddr(Arc::downgrade(child)));
+        self.lifetime.n_children.store(children.len(), Ordering::Relaxed);
     }
 
     fn mark_evicted(&self) {
@@ -373,6 +380,10 @@ impl<S: FlowState> FlowTable<S> {
                 // contention here.
                 let mut children = entry.lifetime.children.write();
                 children.retain(|el| el.0.upgrade().is_some());
+                entry
+                    .lifetime
+                    .n_children
+                    .store(children.len(), Ordering::Relaxed);
                 if !children.is_empty() {
                     return true;
                 }
@@ -430,6 +441,10 @@ impl<S: FlowState> FlowTable<S> {
                 // contention here.
                 let mut children = entry.lifetime.children.write();
                 children.retain(|el| el.0.upgrade().is_some());
+                entry
+                    .lifetime
+                    .n_children
+                    .store(children.len(), Ordering::Relaxed);
                 if !children.is_empty() {
                     return true;
                 }
@@ -521,7 +536,13 @@ impl<S: FlowState> FlowTable<S> {
                 }
                 visited += 1;
 
-                if entry.is_killed() {
+                // We may be visiting this entry before the periodic task is able
+                // to reap it. If so, we have a max-priority candidate, either due
+                // to it losing the support of a crucial LFT or timer expiry.
+                if entry.is_killed()
+                    || (entry.lifetime.n_children.load(Ordering::Relaxed) > 0
+                        && entry.is_expired(now))
+                {
                     to_evict = Some((EvictionKey::Dead, *key, entry));
                     break;
                 }
@@ -678,6 +699,10 @@ struct FlowLifetime {
     /// Whether this flow entry has been explicitly removed.
     killed: AtomicBool,
 
+    /// An estimate of the number of elements in [`Self::children`], used
+    /// to avoid locking and querying consistently empty elements.
+    n_children: AtomicUsize,
+
     /// Entries in remote tables which rely on the continued existence of
     /// this flow.
     ///
@@ -688,10 +713,11 @@ struct FlowLifetime {
 
 impl fmt::Debug for FlowLifetime {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let FlowLifetime { last_hit, killed, children: _ } = self;
+        let FlowLifetime { last_hit, killed, n_children, children: _ } = self;
         f.debug_struct("FlowEntry")
             .field("last_hit", last_hit)
             .field("killed", killed)
+            .field("n_children", n_children)
             .field("children", &"<lock>")
             .finish()
     }
@@ -836,6 +862,7 @@ impl<S: FlowState> FlowEntry<S> {
             lifetime: Arc::new(FlowLifetime {
                 last_hit: Moment::now().raw().into(),
                 killed: false.into(),
+                n_children: 0.into(),
                 children: KRwLock::new(BTreeSet::new()),
             }),
         }