chore(profiler): Use brillig names for outputted flamegraphs

[vezenovm]

Description

Problem*

No issue, just a QOL improvement I found while doing other documentation.

Summary*

We are using the brillig function index when writing a flamegraph file.
e.g. Running noir-profiler opcodes on this program:

fn main(ptr: pub u32, mut array: [u32; 5]) -> pub [u32; 5] {
    // Safety: Sets all elements after `ptr` in `array` to zero.
    let new_array = unsafe { write_to_array(ptr, array) };
    for i in 0..5 {
        if i > ptr {
            assert_eq(new_array[i], 0);
        } else {
            assert_eq(new_array[i], array[i]);
        }
    }
    new_array
}
unconstrained fn write_to_array(ptr: u32, mut array: [u32; 5]) -> [u32; 5] {
    for i in 0..5 {
        if i > ptr {
            array[i] = 0;
        }
    }
    array
}

Will result in the following flamegraph files: main_opcodes.svg, 0_brillig_opcodes.svg, 1_brillig_opcodes.svg, 2_brillig_opcodes.svg. The indices are pretty meaningless without a good amount extra context that is not reasonable to place on users.

Here is how the output directory looks after this PR:

I also changed the flamegraph title:

Before this PR the title would have just been ./target/program.json-brillig_0.

Additional Context

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]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: d506839	Previous: 4e2cd60	Ratio
noir-lang_noir_json_parser_	10 s	8 s	1.25

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[TomAFrench]

How does this handle generic functions or functions with the same name in different modules?

[vezenovm]

How does this handle generic functions or functions with the same name in different modules?

This is going to override repeated names, however, this is already an issue with the ACIR flamegraph (although much more rare as it requires multiple ACIR functions) which uses the function name.

Similarly nargo info just repeats the name:

I'll look at a clean way of representing these function names, although we do lose a lot of the information by the time we get to Brillig/ACIR gen.

For now, I have just pushed a bit of a hack that checks for seen names and generates a suffix to attach to the previously seen function name.

Here is some example output:

EDIT: I also have modified the smoke test to check this functionality.

[jfecher]

For now, I have just pushed a bit of a hack that checks for seen names and generates a suffix to attach to the previously seen function name.

Do we check if the suffix already exists as well? E.g. if we not only have two duplicate write_to_array functions, but also have a write_to_array1 function defined?

KOL improvement

Koality of life? 🐨

[vezenovm]

Do we check if the suffix already exists as well? E.g. if we not only have two duplicate write_to_array functions, but also have a write_to_array1 function defined?

The hack tracks a map of seen names -> suffixes. So it will see multiple duplicates and apply the appropriate suffix. Another example where I use a third duplicate write_to_array:

Koality of life? 🐨

🤣 I was definitely typing too fast. Edited the description

[jfecher]

Another example where I use a third duplicate write_to_array:

I don't mean a third duplicate, I mean still two write_to_array and one function called in source as write_to_array_1 already such that it collides with the default disambiguation strategy. This can potentially happen N times

[vezenovm]

I don't mean a third duplicate, I mean still two write_to_array and one function called in source as write_to_array_1 already such that it collides with the default disambiguation strategy. This can potentially happen N times

Ah I see I read that wrong. Yeah good call out. I'll modify to cover that case.

[jfecher]

LGTM. The while loop could conceptually slow down code but only on inputs where users would go out of their way to name most functions with a numbering scheme. That shouldn't happen in practice so this is fine for a quick tooling feature.

[vezenovm]

LGTM. The while loop could conceptually slow down code but only on inputs where users would go out of their way to name most functions with a numbering scheme. That shouldn't happen in practice so this is fine for a quick tooling feature.

Yeah I figured this case would be quite rare.

[vezenovm]

I switched to the strategy laid out here #7470 (comment).

I was originally trying to recompute a unique index in the profiler. This is unnecessary as a unique index has already been computed for us.