Repository: toml-rs/toml
Pull request #764 (2024-07-29 15:14:48)

perf(parse): Reduce overheasd from "trivia"
`ws_comment_newline` was showing up in callgrind when dealing with a lot of features.  While this had some impact on benchmarks, it wasn't super big.


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 04f6e2df..ad485518 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -104,7 +104,7 @@ jobs:
     - name: Install Rust
       uses: dtolnay/rust-toolchain@stable
       with:
-        toolchain: "1.80"  # STABLE
+        toolchain: "1.79"  # STABLE
     - uses: Swatinem/rust-cache@v2
     - name: Check documentation
       env:
@@ -119,7 +119,7 @@ jobs:
     - name: Install Rust
       uses: dtolnay/rust-toolchain@stable
       with:
-        toolchain: "1.80"  # STABLE
+        toolchain: "1.79"  # STABLE
         components: rustfmt
     - uses: Swatinem/rust-cache@v2
     - name: Check formatting
@@ -135,7 +135,7 @@ jobs:
     - name: Install Rust
       uses: dtolnay/rust-toolchain@stable
       with:
-        toolchain: "1.80"  # STABLE
+        toolchain: "1.79"  # STABLE
         components: clippy
     - uses: Swatinem/rust-cache@v2
     - name: Install SARIF tools
diff --git a/crates/benchmarks/examples/bench.rs b/crates/benchmarks/examples/bench.rs
index 62c748d8..d164e90e 100644
--- a/crates/benchmarks/examples/bench.rs
+++ b/crates/benchmarks/examples/bench.rs
@@ -38,10 +38,7 @@ impl Args {
         let mut parser = Parser::Document;
 
         let mut args = lexopt::Parser::from_env();
-        let mut data = toml_benchmarks::MANIFESTS
-            .iter()
-            .find(|d| d.name() == "1-medium")
-            .unwrap();
+        let mut data_name = "1-medium".to_owned();
         while let Some(arg) = args.next()? {
             match arg {
                 Long("parser") => {
@@ -59,18 +56,18 @@ impl Args {
                     };
                 }
                 Long("manifest") => {
-                    let name = args.value()?;
-                    data = toml_benchmarks::MANIFESTS
-                        .iter()
-                        .find(|d| d.name() == name)
-                        .ok_or_else(|| lexopt::Error::UnexpectedValue {
-                            option: "manifest".to_owned(),
-                            value: name.clone(),
-                        })?;
+                    data_name = args.value()?.string()?;
                 }
                 _ => return Err(arg.unexpected()),
             }
         }
+        let data = toml_benchmarks::MANIFESTS
+            .iter()
+            .find(|d| d.name() == data_name)
+            .ok_or_else(|| lexopt::Error::UnexpectedValue {
+                option: "manifest".to_owned(),
+                value: data_name.into(),
+            })?;
 
         Ok(Self {
             parser,
diff --git a/crates/toml_edit/src/parser/strings.rs b/crates/toml_edit/src/parser/strings.rs
index 0478faa8..cfd5130d 100644
--- a/crates/toml_edit/src/parser/strings.rs
+++ b/crates/toml_edit/src/parser/strings.rs
@@ -328,7 +328,7 @@ fn ml_literal_body<'i>(input: &mut Input<'i>) -> PResult<&'i str> {
 
 // mll-content = mll-char / newline
 fn mll_content(input: &mut Input<'_>) -> PResult<u8> {
-    alt((one_of(MLL_CHAR), newline)).parse_next(input)
+    alt((one_of(MLL_CHAR), newline.value(b'\n'))).parse_next(input)
 }
 
 // mll-char = %x09 / %x20-26 / %x28-7E / non-ascii
diff --git a/crates/toml_edit/src/parser/trivia.rs b/crates/toml_edit/src/parser/trivia.rs
index c62e2dec..919eab78 100644
--- a/crates/toml_edit/src/parser/trivia.rs
+++ b/crates/toml_edit/src/parser/trivia.rs
@@ -1,11 +1,15 @@
 use std::ops::RangeInclusive;
 
 use winnow::combinator::alt;
+use winnow::combinator::empty;
 use winnow::combinator::eof;
+use winnow::combinator::fail;
 use winnow::combinator::opt;
+use winnow::combinator::peek;
 use winnow::combinator::repeat;
 use winnow::combinator::terminated;
 use winnow::prelude::*;
+use winnow::token::any;
 use winnow::token::one_of;
 use winnow::token::take_while;
 
@@ -50,69 +54,60 @@ pub(crate) const NON_EOL: (u8, RangeInclusive<u8>, RangeInclusive<u8>) =
 pub(crate) const COMMENT_START_SYMBOL: u8 = b'#';
 
 // comment = comment-start-symbol *non-eol
-pub(crate) fn comment<'i>(input: &mut Input<'i>) -> PResult<&'i [u8]> {
+pub(crate) fn comment(input: &mut Input<'_>) -> PResult<()> {
     (COMMENT_START_SYMBOL, take_while(0.., NON_EOL))
-        .recognize()
+        .void()
         .parse_next(input)
 }
 
 // newline = ( %x0A /              ; LF
 //             %x0D.0A )           ; CRLF
-pub(crate) fn newline(input: &mut Input<'_>) -> PResult<u8> {
-    alt((
-        one_of(LF).value(b'\n'),
-        (one_of(CR), one_of(LF)).value(b'\n'),
-    ))
+pub(crate) fn newline(input: &mut Input<'_>) -> PResult<()> {
+    dispatch! {any;
+        b'\n' => empty,
+        b'\r' => one_of(LF).void(),
+        _ => fail,
+    }
     .parse_next(input)
 }
 pub(crate) const LF: u8 = b'\n';
 pub(crate) const CR: u8 = b'\r';
 
 // ws-newline       = *( wschar / newline )
-pub(crate) fn ws_newline<'i>(input: &mut Input<'i>) -> PResult<&'i str> {
+pub(crate) fn ws_newline(input: &mut Input<'_>) -> PResult<()> {
     repeat(
         0..,
         alt((newline.value(&b"\n"[..]), take_while(1.., WSCHAR))),
     )
     .map(|()| ())
-    .recognize()
-    .map(|b| unsafe { from_utf8_unchecked(b, "`is_wschar` and `newline` filters out on-ASCII") })
     .parse_next(input)
 }
 
 // ws-newlines      = newline *( wschar / newline )
-pub(crate) fn ws_newlines<'i>(input: &mut Input<'i>) -> PResult<&'i str> {
-    (newline, ws_newline)
-        .recognize()
-        .map(|b| unsafe {
-            from_utf8_unchecked(b, "`is_wschar` and `newline` filters out on-ASCII")
-        })
-        .parse_next(input)
+pub(crate) fn ws_newlines(input: &mut Input<'_>) -> PResult<()> {
+    (newline, ws_newline).void().parse_next(input)
 }
 
 // note: this rule is not present in the original grammar
 // ws-comment-newline = *( ws-newline-nonempty / comment )
-pub(crate) fn ws_comment_newline<'i>(input: &mut Input<'i>) -> PResult<&'i [u8]> {
-    repeat(
-        0..,
-        alt((
-            repeat(
-                1..,
-                alt((take_while(1.., WSCHAR), newline.value(&b"\n"[..]))),
-            )
-            .map(|()| ()),
-            comment.void(),
-        )),
-    )
-    .map(|()| ())
-    .recognize()
+pub(crate) fn ws_comment_newline(input: &mut Input<'_>) -> PResult<()> {
+    let _ = ws.parse_next(input)?;
+
+    dispatch! {opt(peek(any));
+        Some(b'#') => (comment, newline, ws_comment_newline).void(),
+        Some(b'\n') => (newline, ws_comment_newline).void(),
+        Some(b'\r') => (newline, ws_comment_newline).void(),
+        _ => empty,
+    }
     .parse_next(input)
 }
 
 // note: this rule is not present in the original grammar
 // line-ending = newline / eof
-pub(crate) fn line_ending<'i>(input: &mut Input<'i>) -> PResult<&'i str> {
-    alt((newline.value("\n"), eof.value(""))).parse_next(input)
+pub(crate) fn line_ending(input: &mut Input<'_>) -> PResult<()> {
+    alt((newline.value("\n"), eof.value("")))
+        .void()
+        .parse_next(input)
 }
 
 // note: this rule is not present in the original grammar
@@ -151,7 +146,7 @@ mod test {
         ];
         for input in inputs {
             dbg!(input);
-            let parsed = ws_comment_newline.parse(new_input(input));
+            let parsed = ws_comment_newline.recognize().parse(new_input(input));
             assert!(parsed.is_ok(), "{:?}", parsed);
             let parsed = parsed.unwrap();
             assert_eq!(parsed, input.as_bytes());


Test patch:
diff --git a/crates/toml_edit/tests/testsuite/parse.rs b/crates/toml_edit/tests/testsuite/parse.rs
index 62b33bdd..092d3c3d 100644
--- a/crates/toml_edit/tests/testsuite/parse.rs
+++ b/crates/toml_edit/tests/testsuite/parse.rs
@@ -309,13 +309,12 @@ TOML parse error at line 1, column 1
     bad!(
         "a = [ \r ]",
         str![[r#"
-TOML parse error at line 1, column 7
+TOML parse error at line 1, column 8
   |
 1 | a = [ 
  ]
-  |       ^
-invalid array
-expected `]`
+  |        ^
+
 
 "#]]
     );