chore: switch nom to winnow in remaining uses in gix-object, gix-ref, and gix-actor for ~20% more performance.

It's likely that over time, these parsers will get even faster due to improvements to `winnow`.
Thanks, Ed Page, for single-handedly performing this transition.

Author: Byron

Cargo.lock

@@ -34,7 +34,7 @@ toml_edit = "0.19.1"
 semver = "1.0.4"
 crates-index = { version = "2.1.0", default-features = false, features = ["git-performance", "git-https"] }
 cargo_toml = "0.15.1"
-winnow = "0.5.1"
+winnow = "0.5.12"
 git-conventional = "0.12.0"
 time = "0.3.23"
 pulldown-cmark = "0.9.0"

cargo-smart-release/Cargo.lock

@@ -23,7 +23,7 @@ gix-date = { version = "^0.7.1", path = "../gix-date" }
 thiserror = "1.0.38"
 btoi = "0.4.2"
 bstr = { version = "1.3.0", default-features = false, features = ["std", "unicode"]}
-nom = { version = "7", default-features = false, features = ["std"]}
+winnow = { version = "0.5.14", features = ["simd"] }
 itoa = "1.0.1"
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]}
 

cargo-smart-release/Cargo.toml

@@ -1,14 +1,16 @@
 use bstr::ByteSlice;
+use winnow::error::StrContext;
+use winnow::prelude::*;
 
 use crate::{signature::decode, Identity, IdentityRef};
 
 impl<'a> IdentityRef<'a> {
     /// Deserialize an identity from the given `data`.
-    pub fn from_bytes<E>(data: &'a [u8]) -> Result<Self, nom::Err<E>>
+    pub fn from_bytes<E>(mut data: &'a [u8]) -> Result<Self, winnow::error::ErrMode<E>>
     where
-        E: nom::error::ParseError<&'a [u8]> + nom::error::ContextError<&'a [u8]>,
+        E: winnow::error::ParserError<&'a [u8]> + winnow::error::AddContext<&'a [u8], StrContext>,
     {
-        decode::identity(data).map(|(_, t)| t)
+        decode::identity.parse_next(&mut data)
     }
 
     /// Create an owned instance from this shared one.

gix-actor/Cargo.toml

@@ -2,101 +2,75 @@ pub(crate) mod function {
     use bstr::ByteSlice;
     use btoi::btoi;
     use gix_date::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time};
-    use nom::multi::many1_count;
-    use nom::{
-        branch::alt,
-        bytes::complete::{tag, take, take_until, take_while_m_n},
-        character::is_digit,
-        error::{context, ContextError, ParseError},
-        sequence::{terminated, tuple},
-        IResult,
+    use winnow::{
+        combinator::alt,
+        combinator::separated_pair,
+        combinator::terminated,
+        error::{AddContext, ParserError, StrContext},
+        prelude::*,
+        stream::AsChar,
+        token::{take, take_until0, take_while},
     };
-    use std::cell::RefCell;
 
     use crate::{IdentityRef, SignatureRef};
 
     const SPACE: &[u8] = b" ";
 
     /// Parse a signature from the bytes input `i` using `nom`.
-    pub fn decode<'a, E: ParseError<&'a [u8]> + ContextError<&'a [u8]>>(
-        i: &'a [u8],
-    ) -> IResult<&'a [u8], SignatureRef<'a>, E> {
-        use nom::Parser;
-        let tzsign = RefCell::new(b'-'); // TODO: there should be no need for this.
-        let (i, (identity, _, time, _tzsign_count, hours, minutes)) = context(
-            "<name> <<email>> <timestamp> <+|-><HHMM>",
-            tuple((
-                identity,
-                tag(b" "),
-                context("<timestamp>", |i| {
-                    terminated(take_until(SPACE), take(1usize))(i).and_then(|(i, v)| {
-                        btoi::<SecondsSinceUnixEpoch>(v)
-                            .map(|v| (i, v))
-                            .map_err(|_| nom::Err::Error(E::from_error_kind(i, nom::error::ErrorKind::MapRes)))
-                    })
+    pub fn decode<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
+        i: &mut &'a [u8],
+    ) -> PResult<SignatureRef<'a>, E> {
+        separated_pair(
+            identity,
+            b" ",
+            (
+                terminated(take_until0(SPACE), take(1usize))
+                    .verify_map(|v| btoi::<SecondsSinceUnixEpoch>(v).ok())
+                    .context(StrContext::Expected("<timestamp>".into())),
+                alt((
+                    take_while(1.., b'-').map(|_| Sign::Minus),
+                    take_while(1.., b'+').map(|_| Sign::Plus),
+                ))
+                .context(StrContext::Expected("+|-".into())),
+                take_while(2, AsChar::is_dec_digit)
+                    .verify_map(|v| btoi::<OffsetInSeconds>(v).ok())
+                    .context(StrContext::Expected("HH".into())),
+                take_while(1..=2, AsChar::is_dec_digit)
+                    .verify_map(|v| btoi::<OffsetInSeconds>(v).ok())
+                    .context(StrContext::Expected("MM".into())),
+            )
+                .map(|(time, sign, hours, minutes)| {
+                    let offset = (hours * 3600 + minutes * 60) * if sign == Sign::Minus { -1 } else { 1 };
+                    Time {
+                        seconds: time,
+                        offset,
+                        sign,
+                    }
                 }),
-                context(
-                    "+|-",
-                    alt((
-                        many1_count(tag(b"-")).map(|_| *tzsign.borrow_mut() = b'-'), // TODO: this should be a non-allocating consumer of consecutive tags
-                        many1_count(tag(b"+")).map(|_| *tzsign.borrow_mut() = b'+'),
-                    )),
-                ),
-                context("HH", |i| {
-                    take_while_m_n(2usize, 2, is_digit)(i).and_then(|(i, v)| {
-                        btoi::<OffsetInSeconds>(v)
-                            .map(|v| (i, v))
-                            .map_err(|_| nom::Err::Error(E::from_error_kind(i, nom::error::ErrorKind::MapRes)))
-                    })
-                }),
-                context("MM", |i| {
-                    take_while_m_n(1usize, 2, is_digit)(i).and_then(|(i, v)| {
-                        btoi::<OffsetInSeconds>(v)
-                            .map(|v| (i, v))
-                            .map_err(|_| nom::Err::Error(E::from_error_kind(i, nom::error::ErrorKind::MapRes)))
-                    })
-                }),
-            )),
-        )(i)?;
-
-        let tzsign = tzsign.into_inner();
-        debug_assert!(tzsign == b'-' || tzsign == b'+', "parser assure it's +|- only");
-        let sign = if tzsign == b'-' { Sign::Minus } else { Sign::Plus }; //
-        let offset = (hours * 3600 + minutes * 60) * if sign == Sign::Minus { -1 } else { 1 };
-
-        Ok((
-            i,
-            SignatureRef {
-                name: identity.name,
-                email: identity.email,
-                time: Time {
-                    seconds: time,
-                    offset,
-                    sign,
-                },
-            },
-        ))
+        )
+        .context(StrContext::Expected("<name> <<email>> <timestamp> <+|-><HHMM>".into()))
+        .map(|(identity, time)| SignatureRef {
+            name: identity.name,
+            email: identity.email,
+            time,
+        })
+        .parse_next(i)
     }
 
     /// Parse an identity from the bytes input `i` (like `name <email>`) using `nom`.
-    pub fn identity<'a, E: ParseError<&'a [u8]> + ContextError<&'a [u8]>>(
-        i: &'a [u8],
-    ) -> IResult<&'a [u8], IdentityRef<'a>, E> {
-        let (i, (name, email)) = context(
-            "<name> <<email>>",
-            tuple((
-                context("<name>", terminated(take_until(&b" <"[..]), take(2usize))),
-                context("<email>", terminated(take_until(&b">"[..]), take(1usize))),
-            )),
-        )(i)?;
-
-        Ok((
-            i,
-            IdentityRef {
+    pub fn identity<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>(
+        i: &mut &'a [u8],
+    ) -> PResult<IdentityRef<'a>, E> {
+        (
+            terminated(take_until0(&b" <"[..]), take(2usize)).context(StrContext::Expected("<name>".into())),
+            terminated(take_until0(&b">"[..]), take(1usize)).context(StrContext::Expected("<email>".into())),
+        )
+            .map(|(name, email): (&[u8], &[u8])| IdentityRef {
                 name: name.as_bstr(),
                 email: email.as_bstr(),
-            },
-        ))
+            })
+            .context(StrContext::Expected("<name> <<email>>".into()))
+            .parse_next(i)
     }
 }
 pub use function::identity;
@@ -107,12 +81,14 @@ mod tests {
         use bstr::ByteSlice;
         use gix_date::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch};
         use gix_testtools::to_bstr_err;
-        use nom::IResult;
+        use winnow::prelude::*;
 
         use crate::{signature, SignatureRef, Time};
 
-        fn decode(i: &[u8]) -> IResult<&[u8], SignatureRef<'_>, nom::error::VerboseError<&[u8]>> {
-            signature::decode(i)
+        fn decode<'i>(
+            i: &mut &'i [u8],
+        ) -> PResult<SignatureRef<'i>, winnow::error::TreeError<&'i [u8], winnow::error::StrContext>> {
+            signature::decode.parse_next(i)
         }
 
         fn signature(
@@ -132,7 +108,8 @@ mod tests {
         #[test]
         fn tz_minus() {
             assert_eq!(
-                decode(b"Sebastian Thiel <[email protected]> 1528473343 -0230")
+                decode
+                    .parse_peek(b"Sebastian Thiel <[email protected]> 1528473343 -0230")
                     .expect("parse to work")
                     .1,
                 signature("Sebastian Thiel", "[email protected]", 1528473343, Sign::Minus, -9000)
@@ -142,7 +119,8 @@ mod tests {
         #[test]
         fn tz_plus() {
             assert_eq!(
-                decode(b"Sebastian Thiel <[email protected]> 1528473343 +0230")
+                decode
+                    .parse_peek(b"Sebastian Thiel <[email protected]> 1528473343 +0230")
                     .expect("parse to work")
                     .1,
                 signature("Sebastian Thiel", "[email protected]", 1528473343, Sign::Plus, 9000)
@@ -152,7 +130,8 @@ mod tests {
         #[test]
         fn negative_offset_0000() {
             assert_eq!(
-                decode(b"Sebastian Thiel <[email protected]> 1528473343 -0000")
+                decode
+                    .parse_peek(b"Sebastian Thiel <[email protected]> 1528473343 -0000")
                     .expect("parse to work")
                     .1,
                 signature("Sebastian Thiel", "[email protected]", 1528473343, Sign::Minus, 0)
@@ -162,7 +141,8 @@ mod tests {
         #[test]
         fn negative_offset_double_dash() {
             assert_eq!(
-                decode(b"name <[email protected]> 1288373970 --700")
+                decode
+                    .parse_peek(b"name <[email protected]> 1288373970 --700")
                     .expect("parse to work")
                     .1,
                 signature("name", "[email protected]", 1288373970, Sign::Minus, -252000)
@@ -172,30 +152,30 @@ mod tests {
         #[test]
         fn empty_name_and_email() {
             assert_eq!(
-                decode(b" <> 12345 -1215").expect("parse to work").1,
+                decode.parse_peek(b" <> 12345 -1215").expect("parse to work").1,
                 signature("", "", 12345, Sign::Minus, -44100)
             );
         }
 
         #[test]
         fn invalid_signature() {
             assert_eq!(
-                        decode(b"hello < 12345 -1215")
+                        decode.parse_peek(b"hello < 12345 -1215")
                             .map_err(to_bstr_err)
                             .expect_err("parse fails as > is missing")
                             .to_string(),
-                        "Parse error:\nTakeUntil at:  12345 -1215\nin section '<email>', at:  12345 -1215\nin section '<name> <<email>>', at: hello < 12345 -1215\nin section '<name> <<email>> <timestamp> <+|-><HHMM>', at: hello < 12345 -1215\n"
+                        "in slice at ' 12345 -1215'\n  0: expected `<email>` at ' 12345 -1215'\n  1: expected `<name> <<email>>` at ' 12345 -1215'\n  2: expected `<name> <<email>> <timestamp> <+|-><HHMM>` at ' 12345 -1215'\n"
                     );
         }
 
         #[test]
         fn invalid_time() {
             assert_eq!(
-                        decode(b"hello <> abc -1215")
+                        decode.parse_peek(b"hello <> abc -1215")
                             .map_err(to_bstr_err)
                             .expect_err("parse fails as > is missing")
                             .to_string(),
-                        "Parse error:\nMapRes at: -1215\nin section '<timestamp>', at: abc -1215\nin section '<name> <<email>> <timestamp> <+|-><HHMM>', at: hello <> abc -1215\n"
+                        "in predicate verification at 'abc -1215'\n  0: expected `<timestamp>` at 'abc -1215'\n  1: expected `<name> <<email>> <timestamp> <+|-><HHMM>` at 'abc -1215'\n"
                     );
         }
     }

gix-actor/src/identity.rs

@@ -1,15 +1,17 @@
 mod _ref {
     use bstr::ByteSlice;
+    use winnow::error::StrContext;
+    use winnow::prelude::*;
 
     use crate::{signature::decode, IdentityRef, Signature, SignatureRef};
 
     impl<'a> SignatureRef<'a> {
         /// Deserialize a signature from the given `data`.
-        pub fn from_bytes<E>(data: &'a [u8]) -> Result<SignatureRef<'a>, nom::Err<E>>
+        pub fn from_bytes<E>(mut data: &'a [u8]) -> Result<SignatureRef<'a>, winnow::error::ErrMode<E>>
         where
-            E: nom::error::ParseError<&'a [u8]> + nom::error::ContextError<&'a [u8]>,
+            E: winnow::error::ParserError<&'a [u8]> + winnow::error::AddContext<&'a [u8], StrContext>,
         {
-            decode(data).map(|(_, t)| t)
+            decode.parse_next(&mut data)
         }
 
         /// Create an owned instance from this shared one.

gix-actor/src/signature/decode.rs

@@ -9,7 +9,7 @@ fn round_trip() -> gix_testtools::Result {
         b".. whitespace  \t  is explicitly allowed    - unicode aware trimming must be done elsewhere  <[email protected]>"
     ];
     for input in DEFAULTS {
-        let signature: Identity = gix_actor::IdentityRef::from_bytes::<()>(input)?.into();
+        let signature: Identity = gix_actor::IdentityRef::from_bytes::<()>(input).unwrap().into();
         let mut output = Vec::new();
         signature.write_to(&mut output)?;
         assert_eq!(output.as_bstr(), input.as_bstr());