From 236cc1c9ba7f2d3222f48f5ebaf44adcc3250354 Mon Sep 17 00:00:00 2001 From: Joeri Samson Date: Mon, 26 Sep 2022 12:00:35 +0200 Subject: [PATCH 1/2] Use circular instead of slice-deque This probably performs worse, but it removes platform dependent code and it removes all uses of unsafe (which makes me feel better). This is for the moment a straight forward replacement with the minimal changes necessary to make it compile, circular::Buffer has some methods that could be used to tune when the buffer is shifted. We might want to take a look at optimizing the size of the buffer as well. --- Cargo.toml | 2 +- src/reader.rs | 43 ++++++++++++------------------------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 15d8672..01d2e20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ edition = "2021" memchr = "2.2" btoi = "0.4" shakmaty = "0.22" -slice-deque = "0.3" +circular = "0.3" [dev-dependencies] crossbeam = "0.8" diff --git a/src/reader.rs b/src/reader.rs index 170775c..e9f342d 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -24,7 +24,7 @@ use shakmaty::{ san::{San, SanPlus, Suffix}, CastlingSide, Color, Outcome, }; -use slice_deque::SliceDeque; +// use slice_deque::SliceDeque; use crate::{ types::{Nag, RawComment, RawHeader, Skip}, @@ -496,20 +496,20 @@ trait ReadPgn { /// Internal read ahead buffer. #[derive(Debug, Clone)] pub struct Buffer { - inner: SliceDeque, + inner: circular::Buffer, } impl Buffer { fn new() -> Buffer { Buffer { - inner: SliceDeque::with_capacity(MIN_BUFFER_SIZE * 2), + inner: circular::Buffer::with_capacity(MIN_BUFFER_SIZE * 2), } } } impl AsRef<[u8]> for Buffer { fn as_ref(&self) -> &[u8] { - self.inner.as_ref() + self.inner.data() } } @@ -556,17 +556,6 @@ impl BufferedReader { buffer: Buffer::new(), }; - unsafe { - // Initialize the entire ring buffer, so that reading into the - // tail-head slice is always safe. - // - // Use https://doc.rust-lang.org/std/io/struct.Initializer.html - // once stabilized. - let uninitialized = reader.buffer.inner.tail_head_slice(); - assert!(uninitialized.len() >= 2 * MIN_BUFFER_SIZE); - ptr::write_bytes(uninitialized.as_mut_ptr(), 0, uninitialized.len()); - } - reader } @@ -637,24 +626,20 @@ impl ReadPgn for BufferedReader { type Err = io::Error; fn fill_buffer_and_peek(&mut self) -> io::Result> { - while self.buffer.inner.len() < MIN_BUFFER_SIZE { + while self.buffer.inner.available_data() < MIN_BUFFER_SIZE { unsafe { - let size = { - // This is safe because we have initialized the entire - // buffer in the constructor. - let remainder = self.buffer.inner.tail_head_slice(); - self.inner.read(remainder)? - }; + let remainder = self.buffer.inner.space(); + let size = self.inner.read(remainder)?; if size == 0 { break; } - self.buffer.inner.move_tail(size as isize); + self.buffer.inner.fill(size); } } - Ok(self.buffer.inner.front().cloned()) + Ok(self.buffer.inner.data().get(0).cloned()) } fn invalid_data() -> io::Error { @@ -662,19 +647,15 @@ impl ReadPgn for BufferedReader { } fn buffer(&self) -> &[u8] { - self.buffer.inner.as_slice() + self.buffer.inner.data() } fn consume(&mut self, bytes: usize) { - // This is unconditionally safe with a fully initialized buffer. - debug_assert!(bytes <= MIN_BUFFER_SIZE * 2); - unsafe { - self.buffer.inner.move_head(bytes as isize); - } + self.buffer.inner.consume(bytes); } fn peek(&self) -> Option { - self.buffer.inner.front().cloned() + self.buffer.inner.data().get(0).cloned() } } From 6b6ab1a4ad593cb7ef91f36e5cd0b0465c0ed7e9 Mon Sep 17 00:00:00 2001 From: Joeri Samson Date: Tue, 27 Sep 2022 00:52:27 +0200 Subject: [PATCH 2/2] Forgot removing an unnecessary unsafe block --- src/reader.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/reader.rs b/src/reader.rs index e9f342d..127e84b 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -627,16 +627,14 @@ impl ReadPgn for BufferedReader { fn fill_buffer_and_peek(&mut self) -> io::Result> { while self.buffer.inner.available_data() < MIN_BUFFER_SIZE { - unsafe { - let remainder = self.buffer.inner.space(); - let size = self.inner.read(remainder)?; + let remainder = self.buffer.inner.space(); + let size = self.inner.read(remainder)?; - if size == 0 { - break; - } - - self.buffer.inner.fill(size); + if size == 0 { + break; } + + self.buffer.inner.fill(size); } Ok(self.buffer.inner.data().get(0).cloned())