feat(tooling): Skip program transformation when loaded from cache

[aakoshh]

Description

Problem*

Resolves #6669

Summary*

compile_cmd::compile_programs now detects the case when nargo::ops::compile::compile_program has the compiled artefact loaded from cache, rather than recompiled from scratch. When it's loaded from cache, it is assumed to have already gone through the rest of what compile_cmd::compile_programs does, which is to apply nargo::ops::transform_program on it, which includes ACVM optimisations and setting the target expression width. If the current target expression width is the same as the one the package was compiled with, we can skip doing these steps again.

That eliminates the risk of running an non-idempotent optimisation that changes our circuit from what has been persisted before. If somebody were to delete the artefacts and compile from scratch with the same nargo version, then again the transformations were only applied once, which should deterministically yield the same result.

Additional Context

What about compiled contracts?

Curiously compile_cmd::compile_contracts does not look for persisted artefacts, nor does it apply nargo::ops::check_program on the compiled circuits. This is consistent with how noir_wasm does it, but I'm not sure why these steps aren't done, since under the hood there are the same types involved in both cases.

Added Hash to a lot of Brillig types

To support the comparison between cached and compiled artefact I added #[derive(Hash)] to a lot of types that comprise a CompiledProgram. Most of these implemented Eq, so that would have been another option. I didn't want to add a bool flag to the return value of noirc_driver::compile_main to not have a breaking change in the API, although in retrospect I could have added a new method instead. Given that the types seem to implement Eq anyway, I don't see the harm in having Hash as well, and it could be useful in implementing #6670

NB this hash is different than the one we use to decide if we can reuse the artefact, which is based roughly on the AST, and doesn't include the transformations. We don't use the hash to detect whether the transformations result in the same hash, only to decide whether the cached version or a fresh one was returned.

What about Wasm?

The Wasm version doesn't use caching, so it will only apply the transformations once, never repeatedly.

Documentation*

Check one:

• 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 and/or cargo fmt on default settings.

[github-actions]

Peak Memory Sample

Program	Peak Memory
keccak256	83.10M
workspace	122.04M
regression_4709	333.55M
ram_blowup_regression	2.65G

[TomAFrench]

Curiously compile_cmd::compile_contracts does not look for persisted artefacts, nor does it apply nargo::ops::check_program on the compiled circuits.

This is mostly down to keeping things simple compared to hashing all the various contract functions, etc.

[TomAFrench]

This will currently still optimize cached programs in some cases.

Within compile_no_check we use the hash of the monomorphized program rather than all of the input files so that the cache ignores changes in comments, etc. as these don't affect the final output. In the new hash you've added however we're including the debug info which includes the full source which was used to compile the program so this hash will change due to comment changes.

We can then have situations where the "inner" hash check will pass but the "outer" one will not - resulting in the cached program being optimized again.

[aakoshh]

In the new hash you've added however we're including the debug info which includes the full source which was used to compile the program so this hash will change due to comment changes.

Maybe I'm missing something, but this will only happen if the CompiledProgram returned from compile_no_check is not cached_program. If we just return cached_program as-is, then we're going to be returning the cached debug info as well, and the hash will not change. If you change a comment in the program, and it passes the "inner" hash check because no meaningful change has been done, then we still return cached_program, not the one that contains the debug info of the new comment.

We can then have situations where the "inner" hash check will pass but the "outer" one will not

If the "inner" has check passed we return the cached_program without modification, so the "outer" check must pass 🤔

[aakoshh]

What think I'm doing is basically this:

fn compile_package(package) {
    let cached_program = load_cached_program(package);
    let program = compile_program(package, cached_program);
    if program == cached_program {
      return;
    }
    optimize(program);
    save(package, program);
}

[TomAFrench]

Maybe I'm missing something, but this will only happen if the CompiledProgram returned from compile_no_check is not cached_program.

Yeah, this is a brainfart on my end. This shouldn't be an issue.