From f95aafc90e365bbdd243c0ed5069ff23abfa8be4 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Thu, 2 Mar 2023 00:57:54 +0100 Subject: [PATCH] common: allow atomic ringbuf to fill up to N instead of just N-1. This allows the ringbuf to be filled up to `N` instead of just `N-1`, using some fun tricks on the indices. The advantage is better performance: Before, the first write would fill N-1 bytes, The second would write just the 1 byte left before wrapping, then N-2. Then 2, then N-3, and so on. This would result in more smaller chunks, so worse perf. This problem is gone now. --- embassy-hal-common/src/atomic_ring_buffer.rs | 98 ++++++++++++++------ 1 file changed, 69 insertions(+), 29 deletions(-) diff --git a/embassy-hal-common/src/atomic_ring_buffer.rs b/embassy-hal-common/src/atomic_ring_buffer.rs index 4c944d76..ccbc3736 100644 --- a/embassy-hal-common/src/atomic_ring_buffer.rs +++ b/embassy-hal-common/src/atomic_ring_buffer.rs @@ -14,10 +14,18 @@ use core::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; /// One concurrent writer and one concurrent reader are supported, even at /// different execution priorities (like main and irq). pub struct RingBuffer { - buf: AtomicPtr, - len: AtomicUsize, - start: AtomicUsize, - end: AtomicUsize, + pub buf: AtomicPtr, + pub len: AtomicUsize, + + // start and end wrap at len*2, not at len. + // This allows distinguishing "full" and "empty". + // full is when start+len == end (modulo len*2) + // empty is when start == end + // + // This avoids having to consider the ringbuffer "full" at len-1 instead of len. + // The usual solution is adding a "full" flag, but that can't be made atomic + pub start: AtomicUsize, + pub end: AtomicUsize, } pub struct Reader<'a>(&'a RingBuffer); @@ -90,7 +98,7 @@ impl RingBuffer { let start = self.start.load(Ordering::Relaxed); let end = self.end.load(Ordering::Relaxed); - len == 0 || self.wrap(end + 1) == start + self.wrap(start + len) == end } pub fn is_empty(&self) -> bool { @@ -100,15 +108,13 @@ impl RingBuffer { start == end } - fn wrap(&self, n: usize) -> usize { + fn wrap(&self, mut n: usize) -> usize { let len = self.len.load(Ordering::Relaxed); - assert!(n <= len); - if n == len { - 0 - } else { - n + if n >= len * 2 { + n -= len * 2 } + n } } @@ -161,16 +167,25 @@ impl<'a> Writer<'a> { pub fn push_buf(&mut self) -> (*mut u8, usize) { // Ordering: popping writes `start` last, so we read `start` first. // Read it with Acquire ordering, so that the next accesses can't be reordered up past it. - let start = self.0.start.load(Ordering::Acquire); + let mut start = self.0.start.load(Ordering::Acquire); let buf = self.0.buf.load(Ordering::Relaxed); let len = self.0.len.load(Ordering::Relaxed); - let end = self.0.end.load(Ordering::Relaxed); + let mut end = self.0.end.load(Ordering::Relaxed); - let n = if start <= end { - len - end - (start == 0 && len != 0) as usize - } else { - start - end - 1 - }; + let empty = start == end; + + if start >= len { + start -= len + } + if end >= len { + end -= len + } + + if start == end && !empty { + // full + return (buf, 0); + } + let n = if start > end { start - end } else { len - end }; trace!(" ringbuf: push_buf {:?}..{:?}", end, end + n); (unsafe { buf.add(end) }, n) @@ -239,12 +254,23 @@ impl<'a> Reader<'a> { // Ordering: pushing writes `end` last, so we read `end` first. // Read it with Acquire ordering, so that the next accesses can't be reordered up past it. // This is needed to guarantee we "see" the data written by the writer. - let end = self.0.end.load(Ordering::Acquire); + let mut end = self.0.end.load(Ordering::Acquire); let buf = self.0.buf.load(Ordering::Relaxed); let len = self.0.len.load(Ordering::Relaxed); - let start = self.0.start.load(Ordering::Relaxed); + let mut start = self.0.start.load(Ordering::Relaxed); - let n = if end < start { len - start } else { end - start }; + if start == end { + return (buf, 0); + } + + if start >= len { + start -= len + } + if end >= len { + end -= len + } + + let n = if end > start { end - start } else { len - start }; trace!(" ringbuf: pop_buf {:?}..{:?}", start, start + n); (unsafe { buf.add(start) }, n) @@ -280,12 +306,12 @@ mod tests { assert_eq!(rb.is_full(), false); rb.writer().push(|buf| { - // If capacity is 4, we can fill it up to 3. - assert_eq!(3, buf.len()); + assert_eq!(4, buf.len()); buf[0] = 1; buf[1] = 2; buf[2] = 3; - 3 + buf[3] = 4; + 4 }); assert_eq!(rb.is_empty(), false); @@ -301,7 +327,7 @@ mod tests { assert_eq!(rb.is_full(), true); rb.reader().pop(|buf| { - assert_eq!(3, buf.len()); + assert_eq!(4, buf.len()); assert_eq!(1, buf[0]); 1 }); @@ -310,7 +336,7 @@ mod tests { assert_eq!(rb.is_full(), false); rb.reader().pop(|buf| { - assert_eq!(2, buf.len()); + assert_eq!(3, buf.len()); 0 }); @@ -318,11 +344,16 @@ mod tests { assert_eq!(rb.is_full(), false); rb.reader().pop(|buf| { - assert_eq!(2, buf.len()); + assert_eq!(3, buf.len()); assert_eq!(2, buf[0]); assert_eq!(3, buf[1]); 2 }); + rb.reader().pop(|buf| { + assert_eq!(1, buf.len()); + assert_eq!(4, buf[0]); + 1 + }); assert_eq!(rb.is_empty(), true); assert_eq!(rb.is_full(), false); @@ -333,18 +364,27 @@ mod tests { }); rb.writer().push(|buf| { - assert_eq!(1, buf.len()); + assert_eq!(4, buf.len()); buf[0] = 10; 1 }); rb.writer().push(|buf| { - assert_eq!(2, buf.len()); + assert_eq!(3, buf.len()); buf[0] = 11; buf[1] = 12; 2 }); + assert_eq!(rb.is_empty(), false); + assert_eq!(rb.is_full(), false); + + rb.writer().push(|buf| { + assert_eq!(1, buf.len()); + buf[0] = 13; + 1 + }); + assert_eq!(rb.is_empty(), false); assert_eq!(rb.is_full(), true); }