Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Optimize parser #869

Merged
merged 2 commits into from
Feb 17, 2023
Merged

fix: Optimize parser #869

merged 2 commits into from
Feb 17, 2023

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Feb 17, 2023

Related issue(s)

Resolves #791

Description

Optimizes the expression_with_precedence parser which was the main cause of performance lost in the frontend.

The command used for testing was nargo prove p on the source project

fn main(x: bool) {
    constrain x;
}

Timing details are averages of 10 runs and the results are as follows:

           master     this branch
debug      1.758s        0.350s
release    0.470s        0.163s

So it is roughly a 5x speedup in debug and a 2.9x speedup on release.

For a more detailed flamegraph, see the graphs uploaded in #791.

Summary of changes

The old expression_with_precedence parser recursively called itself for each precedence level, but also called each child level twice when constructing expr <op> expr, leading to an exponential blowup. Note though that this is from the time it takes to construct the parser before running it, so this was a large fixed cost rather than one that scaled with program size. This PR simply adds a .clone() to reuse the first parser and make this function linear rather than exponential in time.

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.
  • This PR requires documentation updates when merged.

@jfecher jfecher changed the title Optimize parser fix: Optimize parser Feb 17, 2023
@kevaundray
Copy link
Contributor

Why is this marked as "doc needed"?

@jfecher
Copy link
Contributor Author

jfecher commented Feb 17, 2023

Not sure, it was automatically tagged with that.

@kevaundray kevaundray added this pull request to the merge queue Feb 17, 2023
Merged via the queue into master with commit e927a39 Feb 17, 2023
@kevaundray kevaundray deleted the jf/optimize-parser branch February 17, 2023 18:02
This was referenced Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frontend is slower than necessary
2 participants