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

Feature request: handle a large number of local definitions gracefully #94

Open
lukaszcz opened this issue May 17, 2023 · 3 comments
Open

Comments

@lukaszcz
Copy link
Contributor

lukaszcz commented May 17, 2023

Compiling a file with above ~1000 local definitions results in stack overflow. These kind of files easily result from relatively simple Juvix programs after recursion unrolling (up to ~140 iterations).

test026.pir

@AHartNtkn
Copy link
Collaborator

AHartNtkn commented May 17, 2023

It looks like this overflows during parsing. If this doesn't happen for modules, that suggests that suggests that definitions are parsing let binding in a recursive way that modules aren't.

@AHartNtkn
Copy link
Collaborator

Increasing the stack size to 16 MB, which can be done using the following main function for example

fn main() {
    let builder = thread::Builder::new().stack_size(16 * 1024 * 1024);
    let handler = builder.spawn(move || {
        let cli = Cli::parse();
        match &cli.backend {
            Backend::Plonk(plonk_commands) => plonk(plonk_commands),
            Backend::Halo2(halo2_commands) => halo2(halo2_commands),
        }
    }).unwrap();
    handler.join().unwrap();
}

fixes the stack overflow. I don't know if this solution is desirable in general, but fixing it otherwise would likely require either changing the syntax of the language to make parsing this sort of file less recursive, or switching to a completely different parser that doesn't use recursive parsing at all/as much.

@AHartNtkn
Copy link
Collaborator

I don't know why I didn't test this before, but the stack overflow is also avoided if one uses a release build rather than a dev build. That is,

cargo build --release
./target/release/vamp-ir halo2 compile -s tests/test026.pir -o circuit.halo2

works fine. I wonder how many other inefficiencies are caused by this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants