Merge pull request #3317 from Skgland/optimize-put_back_char

optimize put_back_char

src/machine/dispatch.rs

@@ -1623,7 +1623,7 @@ impl Machine {
                 }
 
                 let Some(inst) = self.code.get(self.machine_st.p) else {
-                    // a seperate function marked #[cold] to make the compiler/branch-predictor prefer the happy path
+                    // a separate function marked #[cold] to make the compiler/branch-predictor prefer the happy path
                     handle_code_index_oob(self.code.len(), self.machine_st.p);
                 };
 
@@ -6038,7 +6038,7 @@ impl Machine {
     }
 }
 
-#[cold] // this is a seperate function so that we can annotate it as cold
+#[cold] // this is a separate function so that we can annotate it as cold
 #[track_caller]
 fn handle_code_index_oob(code_len: usize, p: usize) -> ! {
     panic!("code pointer p = {p} is oob for code area of size {code_len}");

src/parser/char_reader.rs

@@ -102,19 +102,27 @@ impl<R> CharReader<R> {
 }
 
 impl<R: Read> CharReader<R> {
+    pub fn read_chunk(&mut self) -> io::Result<usize> {
+        let mut chunk = [0u8; 8 * 1024];
+        let nread = self.inner.read(&mut chunk)?;
+        self.buf.extend_from_slice(&chunk[..nread]);
+        Ok(nread)
+    }
+
     pub fn refresh_buffer(&mut self) -> io::Result<&[u8]> {
         // If we've reached the end of our internal buffer then we need to fetch
         // some more data from the underlying reader.
         // Branch using `>=` instead of the more correct `==`
         // to tell the compiler that the pos..cap slice is always valid.
         if self.pos >= self.buf.len() {
-            self.buf.clear();
+            // make some space in buf
+            if self.buf.len() > 4 {
+                // keep 4 bytes so that put_back_char can put back at least one char
+                self.buf.drain(4..);
+            }
+            self.pos = self.buf.len();
 
-            let mut chunk = [0u8; 8 * 1024];
+            self.read_chunk()?;
-            let nread = self.inner.read(&mut chunk)?;
-
-            self.buf.extend_from_slice(&chunk[..nread]);
-            self.pos = 0;
         }
 
         Ok(&self.buf[self.pos..])
@@ -135,7 +143,7 @@ impl<R: Read> CharRead for CharReader<R> {
             Err(e) => return Some(Err(e)),
         }
 
-        let bad_bytes_error = |buf: &[u8]| {
+        fn bad_bytes_error(buf: &[u8]) -> std::io::Error {
             // If we have 4 bytes that still don't make up
             // a valid code point, then we have garbage.
 
@@ -143,108 +151,95 @@ impl<R: Read> CharRead for CharReader<R> {
             // leading bytes until either the buffer is
             // empty, or we have a valid code point.
 
-            let mut split_point = 1;
+            // note we might have a sequence of invalid bytes followed by valid bytes followed by invalid bytes
-            let mut badbytes = vec![];
 
-            loop {
+            let err = str::from_utf8(buf).expect_err("the start of buf should be invalid utf-8");
-                let (bad, rest) = buf.split_at(split_point);
+            assert_eq!(err.valid_up_to(), 0, "the error should be a prefix");
 
-                if rest.is_empty() || str::from_utf8(rest).is_ok() {
+            let invalid_prefix = err.error_len().expect("we should have at least 4 bytes");
-                    badbytes.extend_from_slice(bad);
-                    break;
-                }
 
-                split_point += 1;
+            let bad_bytes = buf[..invalid_prefix].to_vec();
-            }
 
-            // Raise the error. If we still have data in
+            io::Error::new(
-            // the buffer, it will be returned on the next
+                io::ErrorKind::InvalidData,
-            // loop.
+                BadUtf8Error { bytes: bad_bytes },
+            )
+        }
 
-            io::Error::new(io::ErrorKind::InvalidData, BadUtf8Error { bytes: badbytes })
+        // while we haven't consumed all bytes from the buffer
-        };
+        while self.pos < self.buf.len() {
-
+            // buf must be non-empty
-        loop {
             let buf = &self.buf[self.pos..];
 
-            if !buf.is_empty() {
+            // we need at most 4 bytes for a char so don't decode the whole buffer
-                let e = match str::from_utf8(buf) {
+            // as it can be quite large and we are going to discard the remaining chars anyway
-                    Ok(s) => {
+            // if there is a valid prefix
-                        let mut chars = s.chars();
+            let prefix = if buf.len() > 4 { &buf[..4] } else { buf };
-                        let c = chars.next().unwrap();
 
-                        return Some(Ok(c));
+            let e = match str::from_utf8(prefix) {
-                    }
+                Ok(s) => {
-                    Err(e) => e,
+                    let mut chars = s.chars();
-                };
+                    let c = chars.next().expect(
+                        "a non-empty buffer that is valid utf-8 contains at least one character",
+                    );
 
-                if buf.len() - e.valid_up_to() >= 4 {
+                    return Some(Ok(c));
-                    return Some(Err(bad_bytes_error(buf)));
+                }
-                } else if self.pos >= self.buf.len() {
+                Err(e) => e,
-                    return None;
+            };
-                } else if self.buf.len() - self.pos >= 4 && self.pos < e.valid_up_to() {
+
-                    return match str::from_utf8(&self.buf[self.pos..self.pos + e.valid_up_to()]) {
+            if e.valid_up_to() != 0 {
-                        Ok(s) => {
+                // the valid prefix is non-empty so it is guaranteed that we can decode at least one char
-                            let mut chars = s.chars();
+                let c = str::from_utf8(&prefix[..e.valid_up_to()])
-                            let c = chars.next().unwrap();
+                    .expect("prefix is verified valid up to this point")
-
+                    .chars()
-                            Some(Ok(c))
+                    .next()
-                        }
+                    .expect("the valid prefix was non-empty");
-                        Err(e) => {
+                return Some(Ok(c));
-                            let badbytes = self.buf[self.pos..self.pos + e.valid_up_to()].to_vec();
+            }
-
+
-                            Some(Err(io::Error::new(
+            if e.error_len().is_some() {
-                                io::ErrorKind::InvalidData,
+                return Some(Err(bad_bytes_error(buf)));
-                                BadUtf8Error { bytes: badbytes },
+            }
-                            )))
+
-                        }
+            // buf is too short to determine if the remaining bytes in buf are a valid char
-                    };
+            // i.e. the content of buf is a prefix of a valid utf-8 encoded char
-                } else {
+            //
-                    let buf_len = self.buf.len();
+            // we need to read more data from the underlying stream
-
+            // so that we can determine its validity
-                    for (c, idx) in (self.pos..buf_len).enumerate() {
+
-                        self.buf[c] = self.buf[idx];
+            if self.buf.len() > 4 {
-                    }
+                // keep a prefix of 4 bytes so that we can put back at least one char
-
+                self.buf.drain(4..self.pos);
-                    self.buf.truncate(buf_len - self.pos);
+                self.pos = 4;
-
+            }
-                    let buf_len = self.buf.len();
+
-                    self.pos = 0;
+            match self.read_chunk() {
-
+                Err(e) => return Some(Err(e)),
-                    if buf_len >= 4 {
+                Ok(0) => return Some(Err(bad_bytes_error(&self.buf))),
-                        continue;
+                Ok(_) => {
-                    }
+                    // successfully filled the buffer with another chunk of data
-
-                    let mut word = [0u8; 4];
-                    let word_slice = &mut word[buf_len..4];
-
-                    match self.inner.read(word_slice) {
-                        Err(e) => return Some(Err(e)),
-                        Ok(0) => return Some(Err(bad_bytes_error(&self.buf))),
-                        Ok(nread) => {
-                            self.buf.extend_from_slice(&word_slice[0..nread]);
-                        }
-                    }
                 }
-            } else {
-                return None;
             }
         }
+
+        None
     }
 
     #[inline(always)]
     fn put_back_char(&mut self, c: char) {
         let c_len = c.len_utf8();
+        if c_len <= self.pos {
+            self.pos -= c_len;
+        } else {
+            self.buf.insert_from_slice(
+                0,
+                &[0u8; 4/* char::MAX_LEN_UTF8 once msrv reached 1.93 */][..c_len - self.pos],
+            );
+            self.pos = 0;
+        }
 
-        let mut shifted_slice = SmallVec::<[u8; 32]>::new();
+        c.encode_utf8(&mut self.buf[self.pos..]);
-        shifted_slice.extend_from_slice(&self.buf[self.pos..]);
-
-        self.buf.clear();
-        self.buf.resize(c_len, 0);
-        c.encode_utf8(&mut self.buf[..c_len]);
-
-        self.buf.extend_from_slice(&shifted_slice);
-        self.pos = 0;
     }
 
     #[inline(always)]
@@ -379,6 +374,51 @@ mod tests {
         assert!(read_string.read_char().is_none());
     }
 
+    #[test]
+    fn interspersed_bad_utf8() {
+        let mut read_string = CharReader::new(Cursor::new(b"a string\xffmore_text\xff"));
+
+        for c in "a string".chars() {
+            assert_eq!(read_string.peek_char().unwrap().ok(), Some(c));
+            assert_eq!(read_string.read_char().unwrap().ok(), Some(c));
+        }
+
+        assert_eq!(
+            read_string.peek_char().unwrap().unwrap_err().kind(),
+            std::io::ErrorKind::InvalidData
+        );
+
+        let err = read_string
+            .read_char()
+            .unwrap()
+            .unwrap_err()
+            .downcast::<BadUtf8Error>()
+            .unwrap();
+
+        read_string.consume(err.bytes.len());
+
+        for c in "more_text".chars() {
+            assert_eq!(read_string.peek_char().unwrap().ok(), Some(c));
+            assert_eq!(read_string.read_char().unwrap().ok(), Some(c));
+        }
+
+        assert_eq!(
+            read_string.peek_char().unwrap().unwrap_err().kind(),
+            std::io::ErrorKind::InvalidData
+        );
+
+        let err = read_string
+            .read_char()
+            .unwrap()
+            .unwrap_err()
+            .downcast::<BadUtf8Error>()
+            .unwrap();
+
+        read_string.consume(err.bytes.len());
+
+        assert!(read_string.read_char().is_none());
+    }
+
     #[test]
     fn greek_string() {
         let mut read_string = CharReader::new(Cursor::new("λέξη"));
@@ -404,7 +444,6 @@ mod tests {
     }
 
     #[test]
-    #[cfg_attr(miri, ignore = "slow and not very relevant")]
     fn greek_lorem_ipsum() {
         let lorem_ipsum = "Λορεμ ιπσθμ δολορ σιτ αμετ, οφφενδιτ
     εφφιcιενδι σιτ ει, ηαρθμ λεγερε qθαερενδθμ ιθσ νε. Ηασ νο εροσ
@@ -476,7 +515,6 @@ mod tests {
     }
 
     #[test]
-    #[cfg_attr(miri, ignore = "slow and not very relevant")]
     fn armenian_lorem_ipsum() {
         let lorem_ipsum = "լոռեմ իպսում դոլոռ սիթ ամեթ, նովում գռաեծո
         սեա եա, աբհոռռեանթ դիսպութանդո եի քուի. իդ քուոդ ինդոծթում
@@ -550,7 +588,6 @@ mod tests {
     }
 
     #[test]
-    #[cfg_attr(miri, ignore = "slow and not very relevant")]
     fn russian_lorem_ipsum() {
         let lorem_ipsum = "Лорем ипсум долор сит амет, атяуи дицам еи
         сит, ид сеа фацилис елаборарет. Меа еу яуас алияуид, те яуи