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

Performance dropping low when pushing arrays to BoundedVec #5013

Closed
Pasifaee opened this issue May 10, 2024 · 2 comments
Closed

Performance dropping low when pushing arrays to BoundedVec #5013

Pasifaee opened this issue May 10, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@Pasifaee
Copy link

Aim

Getting this code to compile considerably fast:

global COUNT = 400;

fn main() -> pub BoundedVec<[Field; 1], COUNT> {
    let mut result = BoundedVec::new();
    let mut x = 0;

    for _ in 0..COUNT {
        if (x < 1) {
            result.push([0]);
        }
    }

    result
}

Expected Behavior

The code compiles fast.

Bug

The code takes 30 seconds to compile on my machine.

To Reproduce

  1. Compile the code

Project Impact

Nice-to-have

Impact Context

No response

Workaround

Yes

Workaround Description

No response

Additional Context

No response

Installation Method

None

Nargo Version

nargo version = 0.28.0 noirc version = 0.28.0+7dfa151da95c7ca9e1aaa752347b62f9fafe2c2c

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@Pasifaee Pasifaee added the bug Something isn't working label May 10, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir May 10, 2024
@jfecher
Copy link
Contributor

jfecher commented May 10, 2024

This runs in under half a second running with nargo e on master. Likewise for proving and verifying.

@jfecher
Copy link
Contributor

jfecher commented May 10, 2024

I've confirmed this takes about 24s for me on commit 7dfa151, so it has been fixed on master since then.

After a git bisect I determined #4716 is the culprit for optimizing this. Although everything was known at compile-time, that PR reduced the number of instructions to merge the arrays in this case from N = 400 each iteration to just the 1 differing element.

Closing this issue - you should see this change once #4716 is included in a release.

@jfecher jfecher closed this as completed May 10, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir May 10, 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
Status: ✅ Done
Development

No branches or pull requests

2 participants