Skip to content
Permalink
Browse files
Improve ParseError::from_error_kind() performance
According to the flamegraph produced by the following command:

  cat large-yarn.nix | cargo flamegraph -p nix-parser --example viewer

The most siginificant amount of time and resources being spent parsing
this very large Nix expression is actually in the error handling code,
specifically in `<Errors as ParseError>::from_error_kind()` and
`<Errors as ParseError>::append()`, mostly in the conversion of the
input fragment into a heap-allocated string for `Error::Nom`. The
specific call dominating the flamegraph was
`core::fmt::write::hbe8b9e0d6296c582`.

Since we are already preserving the span of the input fragment, there
technically is no need to preserve the source text itself since we can
retrieve it later in `Error::to_diagnostic()`.

Removing the `String` payload from the `Error::Nom` variant and removing
all occurrences of `.into()` and `.to_string()` where `Error::Nom`
occurs nets a massive performance win on my MacBook Pro, shown by the
Criterion benchmark below:

```
parse example.nix       time:   [722.02 us 724.88 us 729.24 us]
                        change: [-53.820% -51.759% -48.149%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  3 (3.00%) high mild
  7 (7.00%) high severe
```

This is a reduction from an average time of 1.5558 ms on `cargo bench`.
Additionally, the time spent parsing the `large-yarn.nix` file above was
reduced drastically as well:

```
$ cat large-yarn.nix | time cargo run -p nix-parser --example viewer # Old
        1667.33 real      1262.42 user        10.53 sys

$ cat large-yarn.nix | time cargo run -p nix-parser --example viewer # New
        0.87 real         0.52 user         0.05 sys
```
  • Loading branch information
ebkalderon committed Oct 11, 2019
1 parent d0cfe90 commit 4cd939a2917709a527bd1967f4a29bfd9f2767cc
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 19 deletions.
@@ -133,12 +133,12 @@ where
{
fn from_error_kind(input: I, kind: ErrorKind) -> Self {
Errors {
errors: vec![Error::Nom(input.to_span(), input.to_string(), kind)],
errors: vec![Error::Nom(input.to_span(), kind)],
}
}

fn append(input: I, kind: ErrorKind, mut other: Self) -> Self {
other.push(Error::Nom(input.to_span(), input.to_string(), kind));
other.push(Error::Nom(input.to_span(), kind));
other
}
}
@@ -149,27 +149,18 @@ pub enum Error {
IncorrectDelim(IncorrectDelimError),
UnclosedDelim(UnclosedDelimError),
Unexpected(UnexpectedError),
Nom(Span, String, ErrorKind),
Nom(Span, ErrorKind),
Message(Span, String),
}

impl Error {
pub fn as_nom_error(&self) -> Option<(Span, &str, ErrorKind)> {
match *self {
Error::Nom(ref span, ref frag, ref kind) => Some((*span, frag, *kind)),
_ => None,
}
}
}

impl Display for Error {
fn fmt(&self, fmt: &mut Formatter) -> FmtResult {
match *self {
Error::ExpectedFound(ref e) => write!(fmt, "{}", e),
Error::IncorrectDelim(ref e) => write!(fmt, "{}", e),
Error::UnclosedDelim(ref e) => write!(fmt, "{}", e),
Error::Unexpected(ref e) => write!(fmt, "{}", e),
Error::Nom(_, ref frag, ref e) => write!(fmt, "nom error: (\"{}\", {:?})", frag, e),
Error::Nom(_, ref e) => write!(fmt, "nom error: {:?}", e),
Error::Message(_, ref e) => write!(fmt, "{}", e),
}
}
@@ -208,7 +199,7 @@ impl ToDiagnostic for Error {
Error::IncorrectDelim(ref e) => e.to_diagnostic(file),
Error::Unexpected(ref e) => e.to_diagnostic(file),
Error::UnclosedDelim(ref e) => e.to_diagnostic(file),
Error::Nom(ref span, ref _frag, ref kind) => {
Error::Nom(ref span, ref kind) => {
let label = Label::new(file, *span, self.to_string());
let mut diag = Diagnostic::new_bug(format!("nom error: {:?}", kind), label);
let note = "note: this indicates an unhandled case in the parser".to_string();
@@ -24,11 +24,7 @@ pub fn float(input: LocatedSpan) -> IResult<Token> {
Ok((remaining, Token::Float(float, span.to_span())))
} else {
let mut errors = Errors::new();
errors.push(Error::Nom(
input.to_span(),
input.fragment.into(),
ErrorKind::Float,
));
errors.push(Error::Nom(input.to_span(), ErrorKind::Float));
Err(nom::Err::Error(errors))
}
}

0 comments on commit 4cd939a

Please sign in to comment.