Repository: pest-parser/pest (language: rust)

Issue:
perf regression in 2.7.9
**Describe the bug**
Huge perf regression in 2.7.9, probably because of #965, visible in `http parser` and `json parser` benchmarks

**To Reproduce**
checkout 2.7.8 version of pest, cd grammars, cargo bench
checkout 2.7.9 version of pest, cd grammars, cargo bench
see the diff

**Expected behavior**
Not big regressions like here:
```
http parser             time:   [3.5245 ms 3.5328 ms 3.5464 ms]
                        change: [+1003.6% +1008.2% +1014.4%] (p = 0.00 < 0.05)
                        Performance has regressed.

json parser             time:   [215.12 µs 215.53 µs 216.20 µs]
                        change: [+919.13% +928.78% +940.25%] (p = 0.00 < 0.05)
                        Performance has regress
```

**Additional context**
One of probable slowdown spots?
https://github.com/pest-parser/pest/blob/f60b5184e5eb1f6ef3486614bab6ff89f359ed39/pest/src/parser_state.rs#L1019-L1021



Discussion / hints:
@EmirVildanov do you think that can be optimized? Maybe the allocation can be removed if the token is &str instead of String?
Alternatively we can perhaps have this better error reporting as a compile time or runtime option
I am also noticed this slowdown. It's *very* noticeable in real-world situations.

In a private development of branch [rustray](https://github.com/chrivers/rustray), I noticed this in the SBT2-parser (for ray tracer scene files).

With pest version 2.7.9:

```
[*]   parse .....:  2139.15 ms
```

Downgrading to 2.7.8 yields:

```
[*]   parse .....:   297.14 ms
```

So yeah, about 750% slower :-/
I'm guessing the main culprit may be `try_add_new_stack_rule` which is called often during parsing and the maintainance of that call stack may be heavy: https://github.com/pest-parser/pest/pull/965/files#diff-4bc69b84e1294b9c791fd94baf98de34748c484129ac9a6c2757babefcf862e0R274
> @EmirVildanov do you think that can be optimized? Maybe the allocation can be removed if the token is &str instead of String?

Yeah, I'll profile the code and try to optimize it!
Thanks for pointing at the possible culprit and sorry for introduced regression 🙏 .
> Yeah, I'll profile the code and try to optimize it!
Thanks for pointing at the possible culprit and sorry for introduced regression 🙏 .

I tried to reproduce it and indeed, there's a regression. In my setup (M1 CPU), it was approx. 50% faster than what @klensy posted:
```
http parser             time:   [2.0941 ms 2.0948 ms 2.0960 ms]
                        change: [+500.10% +520.67% +538.10%] (p = 0.00 < 0.05)
                        Performance has regressed.

json parser             time:   [124.61 µs 124.65 µs 124.71 µs]
                        change: [+492.54% +492.97% +493.40%] (p = 0.00 < 0.05)
                        Performance has regressed.
```

but it's still a regression / around 5x slower than the previous version baseline.

Without profiling, a few optimization ideas besides reducing the allocations:
- not clearing those tracking structures where possible (and instead keeping track of where last position tokens were added)
- different data structures for lookups: e.g. instead of `call_stack in self.call_stacks.iter().skip(start_index)` to find the deepest rule etc., having a few refs or a HashMap (or BTreeMap) to look it up

But if optimizations can't bring the overhead down to be close to the previous baseline, I guess the best course of action may be to introduce a runtime `AtomicBool` flag that one can set if one wants better error reporting? I assume this is better as a runtime option (instead of a compile-time), as one may want to do potentially both: false for default parsing that has a similar performance behaviour as the previous version; true if one wants to explicitly get a better error report?

Fix patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 04abff93..ecb75f4f 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -79,7 +79,7 @@ jobs:
         with:
           kind: msrv
           tools: cargo-msrv
-          toolchain: 1.70.0 # needed for cargo-msrv due to cargo-platform v0.1.6
+          toolchain: 1.73.0 # needed for cargo-msrv due to cargo-platform v0.1.8
       - name: Check msrv
         shell: sh
         run: for crate in "derive" "generator" "grammars" "meta" "pest" "vm"; do cd "$crate" && cargo msrv verify && cd ..; done
diff --git a/debugger/Cargo.toml b/debugger/Cargo.toml
index 8819f549..e87337c4 100644
--- a/debugger/Cargo.toml
+++ b/debugger/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest_debugger"
 description = "pest grammar debugger"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = [
     "Dragoș Tiselice <dragostiselice@gmail.com>",
@@ -17,9 +17,9 @@ readme = "_README.md"
 rust-version = "1.61"
 
 [dependencies]
-pest = { path = "../pest", version = "2.7.9" }
-pest_meta = { path = "../meta", version = "2.7.9" }
-pest_vm = { path = "../vm", version = "2.7.9" }
+pest = { path = "../pest", version = "2.7.10" }
+pest_meta = { path = "../meta", version = "2.7.10" }
+pest_vm = { path = "../vm", version = "2.7.10" }
 reqwest = { version = "= 0.11.13", default-features = false, features = [
     "blocking",
     "json",
diff --git a/derive/Cargo.toml b/derive/Cargo.toml
index ace3c4c0..dbdfb25c 100644
--- a/derive/Cargo.toml
+++ b/derive/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest_derive"
 description = "pest's derive macro"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = ["Dragoș Tiselice <dragostiselice@gmail.com>"]
 homepage = "https://pest.rs/"
@@ -25,5 +25,5 @@ grammar-extras = ["pest_generator/grammar-extras"]
 
 [dependencies]
 # for tests, included transitively anyway
-pest = { path = "../pest", version = "2.7.9", default-features = false }
-pest_generator = { path = "../generator", version = "2.7.9", default-features = false }
+pest = { path = "../pest", version = "2.7.10", default-features = false }
+pest_generator = { path = "../generator", version = "2.7.10", default-features = false }
diff --git a/generator/Cargo.toml b/generator/Cargo.toml
index fa16808b..77b5196c 100644
--- a/generator/Cargo.toml
+++ b/generator/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest_generator"
 description = "pest code generator"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = ["Dragoș Tiselice <dragostiselice@gmail.com>"]
 homepage = "https://pest.rs/"
@@ -22,8 +22,8 @@ grammar-extras = ["pest_meta/grammar-extras"]
 export-internal = []
 
 [dependencies]
-pest = { path = "../pest", version = "2.7.9", default-features = false }
-pest_meta = { path = "../meta", version = "2.7.9" }
+pest = { path = "../pest", version = "2.7.10", default-features = false }
+pest_meta = { path = "../meta", version = "2.7.10" }
 proc-macro2 = "1.0"
 quote = "1.0"
 syn = "2.0"
diff --git a/grammars/Cargo.toml b/grammars/Cargo.toml
index 6224c9f2..0b5393cc 100644
--- a/grammars/Cargo.toml
+++ b/grammars/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest_grammars"
 description = "pest popular grammar implementations"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = ["Dragoș Tiselice <dragostiselice@gmail.com>"]
 homepage = "https://pest.rs/"
@@ -14,8 +14,8 @@ readme = "_README.md"
 rust-version = "1.61"
 
 [dependencies]
-pest = { path = "../pest", version = "2.7.9" }
-pest_derive = { path = "../derive", version = "2.7.9" }
+pest = { path = "../pest", version = "2.7.10" }
+pest_derive = { path = "../derive", version = "2.7.10" }
 
 [dev-dependencies]
 criterion = "0.5"
diff --git a/grammars/src/lib.rs b/grammars/src/lib.rs
index 94c26694..0e7ae42e 100644
--- a/grammars/src/lib.rs
+++ b/grammars/src/lib.rs
@@ -279,6 +279,8 @@ mod tests {
 
     #[test]
     fn sql_parse_attempts_error() {
+        pest::set_error_detail(true);
+
         fn is_whitespace(string: String) -> bool {
             string == "\r\n"
                 || (string.len() == 1 && string.chars().next().unwrap().is_whitespace())
diff --git a/meta/Cargo.toml b/meta/Cargo.toml
index 535ed115..30927647 100644
--- a/meta/Cargo.toml
+++ b/meta/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest_meta"
 description = "pest meta language parser and validator"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = ["Dragoș Tiselice <dragostiselice@gmail.com>"]
 homepage = "https://pest.rs/"
@@ -22,7 +22,7 @@ include = [
 rust-version = "1.61"
 
 [dependencies]
-pest = { path = "../pest", version = "2.7.9" }
+pest = { path = "../pest", version = "2.7.10" }
 once_cell = "1.8.0"
 
 [build-dependencies]
diff --git a/pest/Cargo.toml b/pest/Cargo.toml
index eec4c9b2..508b1b83 100644
--- a/pest/Cargo.toml
+++ b/pest/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest"
 description = "The Elegant Parser"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = ["Dragoș Tiselice <dragostiselice@gmail.com>"]
 homepage = "https://pest.rs/"
diff --git a/pest/src/lib.rs b/pest/src/lib.rs
index 2ec2688c..0dde153f 100644
--- a/pest/src/lib.rs
+++ b/pest/src/lib.rs
@@ -336,7 +336,8 @@ extern crate std;
 
 pub use crate::parser::Parser;
 pub use crate::parser_state::{
-    set_call_limit, state, Atomicity, Lookahead, MatchDir, ParseResult, ParserState,
+    set_call_limit, set_error_detail, state, Atomicity, Lookahead, MatchDir, ParseResult,
+    ParserState,
 };
 pub use crate::position::Position;
 pub use crate::span::{merge_spans, Lines, LinesSpan, Span};
diff --git a/pest/src/parser_state.rs b/pest/src/parser_state.rs
index 1193eb33..276e81f5 100644
--- a/pest/src/parser_state.rs
+++ b/pest/src/parser_state.rs
@@ -20,7 +20,7 @@ use alloc::vec::Vec;
 use core::fmt::{Debug, Display, Formatter};
 use core::num::NonZeroUsize;
 use core::ops::Range;
-use core::sync::atomic::{AtomicUsize, Ordering};
+use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 
 use crate::error::{Error, ErrorVariant};
 use crate::iterators::pairs::new;
@@ -103,6 +103,22 @@ pub fn set_call_limit(limit: Option<NonZeroUsize>) {
     CALL_LIMIT.store(limit.map(|f| f.get()).unwrap_or(0), Ordering::Relaxed);
 }
 
+static ERROR_DETAIL: AtomicBool = AtomicBool::new(false);
+
+/// Sets whether information for more error details
+/// should be collected. This is useful for debugging
+/// parser errors (as it leads to more comprehensive
+/// error messages), but it has a higher performance cost.
+/// (hence, it's off by default)
+///
+/// # Arguments
+///
+/// * `enabled` - Whether to enable the collection for
+///               more error details.
+pub fn set_error_detail(enabled: bool) {
+    ERROR_DETAIL.store(enabled, Ordering::Relaxed);
+}
+
 #[derive(Debug)]
 struct CallLimitTracker {
     current_call_limit: Option<(usize, usize)>,
@@ -204,6 +220,8 @@ impl Display for ParsingToken {
 /// The intuition is such rules will be most likely the query user initially wanted to write.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct ParseAttempts<R> {
+    /// Indicates whether the parsing attempts are tracked.
+    enabled: bool,
     /// Vec of rule calls sequences awaiting tokens at the same `max_position`.
     /// If there are several stacks in vec, it means all those rule stacks are "equal"
     /// because their attempts occurred on the same position.
@@ -227,6 +245,7 @@ impl<R: RuleType> ParseAttempts<R> {
             expected_tokens: Vec::with_capacity(EXPECTED_TOKENS_INITIAL_CAPACITY),
             unexpected_tokens: Vec::with_capacity(EXPECTED_TOKENS_INITIAL_CAPACITY),
             max_position: 0,
+            enabled: ERROR_DETAIL.load(Ordering::Relaxed),
         }
     }
 
@@ -461,11 +480,18 @@ where
                 }
             };
 
-            Err(Error::new_from_pos_with_parsing_attempts(
-                variant,
-                Position::new_internal(input, state.attempt_pos),
-                state.parse_attempts.clone(),
-            ))
+            if state.parse_attempts.enabled {
+                Err(Error::new_from_pos_with_parsing_attempts(
+                    variant,
+                    Position::new_internal(input, state.attempt_pos),
+                    state.parse_attempts.clone(),
+                ))
+            } else {
+                Err(Error::new_from_pos(
+                    variant,
+                    Position::new_internal(input, state.attempt_pos),
+                ))
+            }
         }
     }
 }
@@ -675,7 +701,9 @@ impl<'i, R: RuleType> ParserState<'i, R> {
                 // Note, that we need to count positive parsing results too, because we can fail in
                 // optional rule call inside which may lie the farthest
                 // parsed token.
-                try_add_rule_to_stack(&mut new_state);
+                if new_state.parse_attempts.enabled {
+                    try_add_rule_to_stack(&mut new_state);
+                }
                 Ok(new_state)
             }
             Err(mut new_state) => {
@@ -687,7 +715,9 @@ impl<'i, R: RuleType> ParserState<'i, R> {
                         neg_attempts_index,
                         attempts,
                     );
-                    try_add_rule_to_stack(&mut new_state);
+                    if new_state.parse_attempts.enabled {
+                        try_add_rule_to_stack(&mut new_state);
+                    }
                 }
 
                 if new_state.lookahead == Lookahead::None
@@ -981,13 +1011,15 @@ impl<'i, R: RuleType> ParserState<'i, R> {
     where
         F: FnOnce(char) -> bool,
     {
-        let token = ParsingToken::BuiltInRule;
         let start_position = self.position.pos();
-        if self.position.match_char_by(f) {
-            self.handle_token_parse_result(start_position, token, true);
+        let succeeded = self.position.match_char_by(f);
+        if self.parse_attempts.enabled {
+            let token = ParsingToken::BuiltInRule;
+            self.handle_token_parse_result(start_position, token, succeeded);
+        }
+        if succeeded {
             Ok(self)
         } else {
-            self.handle_token_parse_result(start_position, token, false);
             Err(self)
         }
     }
@@ -1016,15 +1048,17 @@ impl<'i, R: RuleType> ParserState<'i, R> {
     /// ```
     #[inline]
     pub fn match_string(mut self: Box<Self>, string: &str) -> ParseResult<Box<Self>> {
-        let token = ParsingToken::Sensitive {
-            token: String::from(string),
-        };
         let start_position = self.position.pos();
-        if self.position.match_string(string) {
-            self.handle_token_parse_result(start_position, token, true);
+        let succeeded = self.position.match_string(string);
+        if self.parse_attempts.enabled {
+            let token = ParsingToken::Sensitive {
+                token: String::from(string),
+            };
+            self.handle_token_parse_result(start_position, token, succeeded);
+        }
+        if succeeded {
             Ok(self)
         } else {
-            self.handle_token_parse_result(start_position, token, false);
             Err(self)
         }
     }
@@ -1053,15 +1087,17 @@ impl<'i, R: RuleType> ParserState<'i, R> {
     /// ```
     #[inline]
     pub fn match_insensitive(mut self: Box<Self>, string: &str) -> ParseResult<Box<Self>> {
-        let token = ParsingToken::Insensitive {
-            token: String::from(string),
-        };
-        let start_position = self.position().pos();
-        if self.position.match_insensitive(string) {
-            self.handle_token_parse_result(start_position, token, true);
+        let start_position: usize = self.position().pos();
+        let succeeded = self.position.match_insensitive(string);
+        if self.parse_attempts.enabled {
+            let token = ParsingToken::Insensitive {
+                token: String::from(string),
+            };
+            self.handle_token_parse_result(start_position, token, succeeded);
+        }
+        if succeeded {
             Ok(self)
         } else {
-            self.handle_token_parse_result(start_position, token, false);
             Err(self)
         }
     }
@@ -1093,16 +1129,18 @@ impl<'i, R: RuleType> ParserState<'i, R> {
     /// ```
     #[inline]
     pub fn match_range(mut self: Box<Self>, range: Range<char>) -> ParseResult<Box<Self>> {
+        let start_position = self.position().pos();
         let token = ParsingToken::Range {
             start: range.start,
             end: range.end,
         };
-        let start_position = self.position().pos();
-        if self.position.match_range(range) {
-            self.handle_token_parse_result(start_position, token, true);
+        let succeeded = self.position.match_range(range);
+        if self.parse_attempts.enabled {
+            self.handle_token_parse_result(start_position, token, succeeded);
+        }
+        if succeeded {
             Ok(self)
         } else {
-            self.handle_token_parse_result(start_position, token, false);
             Err(self)
         }
     }
diff --git a/vm/Cargo.toml b/vm/Cargo.toml
index 493557a1..f8c8e550 100644
--- a/vm/Cargo.toml
+++ b/vm/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name = "pest_vm"
 description = "pest grammar virtual machine"
-version = "2.7.9"
+version = "2.7.10"
 edition = "2021"
 authors = ["Dragoș Tiselice <dragostiselice@gmail.com>"]
 homepage = "https://pest.rs/"
@@ -14,8 +14,8 @@ readme = "_README.md"
 rust-version = "1.61"
 
 [dependencies]
-pest = { path = "../pest", version = "2.7.9" }
-pest_meta = { path = "../meta", version = "2.7.9" }
+pest = { path = "../pest", version = "2.7.10" }
+pest_meta = { path = "../meta", version = "2.7.10" }
 
 [features]
 grammar-extras = ["pest_meta/grammar-extras"]


Test patch: