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.

Sign up for GitHub

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

Jump to bottom

Investigate parsing all modules in a separate stage #3838

Closed
kevaundray opened this issue Dec 16, 2023 · 0 comments · Fixed by #4063
Closed

Investigate parsing all modules in a separate stage #3838

kevaundray opened this issue Dec 16, 2023 · 0 comments · Fixed by #4063
Assignees
@sirasistant
Labels
enhancement New feature or request
Milestone
0.24

Comments

@kevaundray
Copy link
Contributor

Problem

This is similar to the change made to the file manager which made it read-only.

We would parse all Noir files as a separate pass and then resolution would lookup the parsed AST's in a read-only map.

The motivation is to move towards an arhcitecture where we can clearly separate and cache different stages of compilation. Whether this can be done for all stages in the frontend is to be decided.

Happy Case

.

Alternatives Considered

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response
@kevaundray kevaundray added enhancement New feature or request P-LOW labels Dec 16, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Dec 16, 2023
@kevaundray kevaundray added this to the 0.24.0 milestone Jan 15, 2024
@sirasistant sirasistant mentioned this issue Jan 17, 2024
Merged
5 tasks
github-merge-queue bot pushed a commit that referenced this issue Jan 18, 2024
@sirasistant @kobyhallx
feat: Extract parsing to its own pass and do it in parallel (#4063)
569cbbc 
# Description
Rework of #3849 after
file_manager_with_stdlib was created. Also as @kevaundray suggested
extracted the parsing above the context.

Resolves #3838
## Problem\*
Parsing is currently done when collecting, allowing only to parse files
used, but making it very difficult to do parallel parsing or to have
cached parsing.

## Summary\*

This PR extracts parsing to its own pass, and makes the Context take the
parsed files. The creator of the context is the one in charge to do the
parsing of the file manager, so Nargo, the LSP and wasm need to handle
the parsed files. This PR uses rayon to do parallel parsing in Nargo &
LSP. It reduces the time taken to process the on save notification in
the LSP from ~700ms on protocol circuits to ~250ms.

With parsing being in its own pass, this opens the door for the LSP to
cache parsing. It has access to the file manager and the parsed files,
so it can detect which files have changed on disk and only parse those
when necessary.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Koby Hall <102518238+kobyhallx@users.noreply.github.com>
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sirasistant sirasistant

Labels
enhancement New feature or request
Projects
Noir
Archived in project
Milestone
0.24
Development

Successfully merging a pull request may close this issue.

feat: Extract parsing to its own pass and do it in parallel noir-lang/noir
3 participants
@sirasistant @kevaundray @Savio-Sou