chore(docs): Touch up profiler docs #7524
Conversation
Savio-Sou
added
documentation
|
🚀 Deployed on https://67d1bbf110d5030a155f22ca--noir-docs.netlify.app |
|
|
||
| ### Generating an ACIR opcode flamegraph | ||
| #### Flamegraphing |
There was a problem hiding this comment.
we should add this word to cspell
|
|
||
| ::: | ||
|
|
||
| ### Profiling execution traces (unconstrained) |
There was a problem hiding this comment.
I do greatly prefer to keep the flow of ACIR opcodes -> backend gates -> Brillig flamegraphs.
I foresee most devs using the ACIR/backend gates profiling to reduce their circuit proving speeds over witness generation speeds. Backend gates also do not yet require introducing a Prover.toml, and unconstrained. Due to these reasons, I feel the backend gates section naturally follows the ACIR profiling section.
There was a problem hiding this comment.
I was debating if that's the right change as well.
I agree that devs' interests lie primarily with profiling backend gates; if anything, backend gates > execution > ACIR opcodes.
There was a problem hiding this comment.
The question + my assumption that drove the change though was:
Do we intend to maintain backend gates profiling within noir-lang, and extend support for different backends?
I naively assumed no; but now that I think of it, maybe it's just a simple interface where backends will do the heavy lifting?
There was a problem hiding this comment.
Do we intend to maintain backend gates profiling within noir-lang, and extend support for different backends?
Backends do the heavy lifting. The profiler simply accepts a path to a backend binary, the name of the their gates command, and allows to pass optional arguments (we do not document this here as it is not relevant for bb). Backends just need to follow our gates API if they want to integrate with the profiler.
There was a problem hiding this comment.
Lovely! I will update the ordering to Backend Gates > Brillig > ACIR in that case 👍 lmk if you have concerns.
There was a problem hiding this comment.
I still prefer the ordering ACIR > Backend gates > Brillig as mentioned in the original comment. I think it flows better to keep the circuit flamegraphs together, as the backend flamegraph essentially builds upon the acir flamegraph (we still display acir opcodes in our backend flamegraph, just the sample count associated with those opcodes has now changed).
There was a problem hiding this comment.
Is profiling ACIR alone useful for devs?
If not, maybe we can also consider merging the ACIR and Backend Gates sections; wdyt
There was a problem hiding this comment.
Is profiling ACIR alone useful for devs?
Not sure what you mean by "merging" the sections. Do you mean removing the ACIR section? I do think they warrant separate sections as profiling ACIR is useful. Plus it is Noir's compilation target, so I think it should be documented how to profile it.
It provides additional context as ACIR opcodes counts do not tie directly to backend gate counts. e.g. in this guide's example code backend gates from range opcodes take up most of the program, but it doesn't give us a full picture.
If we wanted to optimize witness generation we would need to look at the ACIR as well. Looking at the backend gates provides optimizations for proving speed. I realize this is why you wanted to go Backend Gates > Brillig > ACIR, but I think we should still go ACIR > Backend gates > Brillig for the reasons mentioned above.
There was a problem hiding this comment.
Should we also hover over "all" to show the total gates in this image?
|  | ||
| Flamegraph of the demonstrative project generated with bb v0.76.4: | ||
|
|
||
|  |
There was a problem hiding this comment.
Should we show the unoptimized one before this?
There was a problem hiding this comment.
Bumped the optimized flamegraph as this reads after the optimizing section.
Happy to reposition according to any reordering of sections as a result of the parallel discussion above.
Co-authored-by: Maxim Vezenov <[email protected]>
|
FYI @noir-lang/developerrelations on Noir doc changes. |
Description
Summary*
execution-opcodesflag names documentedAdditional Context
Docusaurus seems to support inlining SVGs, might be worth exploring as a future endeavor.
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.