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

OOM when reading from a single large constant array many times #4709

Closed
TomAFrench opened this issue Apr 3, 2024 · 2 comments · Fixed by #4923
Closed

OOM when reading from a single large constant array many times #4709

TomAFrench opened this issue Apr 3, 2024 · 2 comments · Fixed by #4923
Assignees
Labels
bug Something isn't working
Milestone

Comments

@TomAFrench
Copy link
Member

See the linked repo here: https://github.com/AndreiCravtov/noir-crypto-kit/blob/d7db6bc93f60709abad4bda3b810e900b614d068/noir/src/main.nr

This program reads values from a large constant array at constant indices to calculate a constant value acc, however it crashes once we reach the loop unrolling phase.

My assumption is that we're creating many unnecessary duplicates of the array contents, we then get P*P copies of a P*P array which would take horrendous amounts of RAM to store. If this is the case then we should look at ways to deduplicate constant arrays similarly to how we handle constant numeric values.

@TomAFrench TomAFrench added the bug Something isn't working label Apr 3, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Apr 3, 2024
@TomAFrench
Copy link
Member Author

This is a blocker for using arrays as lookup tables of any considerable size.

@jfecher
Copy link
Contributor

jfecher commented Apr 10, 2024

My assumption is that we're creating many unnecessary duplicates of the array contents

Yeah, this is because globals are "inlined" during monomorphization when they're used. Maybe we can add a quick fix to remember the global's ID and cache its value per-function instead.

@TomAFrench TomAFrench added this to the 1.0 milestone Apr 16, 2024
@TomAFrench TomAFrench moved this from 📋 Backlog to 🏗 In progress in Noir Apr 26, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 26, 2024
# Description

## Problem\*

Resolves #4709 

## Summary\*
The issue is due to reference count instructions which blow-up during
unrolling. Since these instructions are only for Brillig and since loops
are not unrolled in Brillig, I simply discard them during unrolling.


## Additional Context
I did not get an OOM on the mainframe, but the compilation lasted many
hours and did not complete (I finally cancelled it). With the fix the
execution is instantaneous, whether the code is constrained or
unconstrained.


## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

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

---------

Co-authored-by: Tom French <[email protected]>
Co-authored-by: jfecher <[email protected]>
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Noir Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants