feat: Add reusable procedures to brillig generation#7981
Conversation
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Proof generationEach column represents the number of threads used in proof generation.
L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 8 txs.
Circuits statsStats on running time and I/O sizes collected for every kernel circuit run across all benchmarks.
Stats on running time collected for app circuits
AVM SimulationTime to simulate various public functions in the AVM.
Public DB AccessTime to access various public DBs.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method | Metric | | |
| }); | ||
| } | ||
|
|
||
| /// Computes the size of a parameter if it was flattened |
There was a problem hiding this comment.
All of these moved from entry_point.rs, since they are not only used in the entrypoint
TomAFrench
left a comment
TomAFrench
left a comment
There was a problem hiding this comment.
Small review, will look on monday.
| /// Returns the usize one register. This will be used to perform arithmetic operations. | ||
| pub(crate) fn usize_one() -> MemoryAddress { | ||
| MemoryAddress::from(ReservedRegisters::UsizeOne as usize) | ||
| } |
There was a problem hiding this comment.
I've noticed a couple of places where we'd benefit from a usize_zero reserved register as well (functions with lots of multiplications, etc)
There was a problem hiding this comment.
Ah this isn't a pure usize as we'd need one for each bitsize which the brillig VM can address. Would still be a saving in general though (at least for commonly used bit sizes.)
There was a problem hiding this comment.
Yeah for the case of zero it's not used as much as usize_one in one specific bitsize, the only downside with having these reserved registers with commonly used values is that they have to be initialized in the entrypoint, presenting a constant cost for programs that don't use them. Maybe if the linker could collect which ones are actually used and only initialize those...
There was a problem hiding this comment.
Yeah, I think this is worth exploring but we can do this separately (and in the noir repo!)
There was a problem hiding this comment.
Yup, it'd be pretty good, we could always reserve the registers and initialize lazily. It should help!
There was a problem hiding this comment.
The downside of doing this kind of optimization work in the noir repo is that it's a pain to get bytecode size metrics there for the aztec contracts
There was a problem hiding this comment.
Soon we will have a gates diff like we do for ACIR noir-lang/noir#5747 (comment) that we can utilize for testing specific cases. This isn't the aztec contracts, but we can then see the improvements on a sync.
| /// Returns the usize one register. This will be used to perform arithmetic operations. | ||
| pub(crate) fn usize_one() -> MemoryAddress { | ||
| MemoryAddress::from(ReservedRegisters::UsizeOne as usize) | ||
| } |
There was a problem hiding this comment.
Yeah, I think this is worth exploring but we can do this separately (and in the noir repo!)
| // Only deallocate step if we allocated it | ||
| if step.is_none() { | ||
| self.deallocate_register(step_register); | ||
| } |
There was a problem hiding this comment.
We're now leaking a register in the case where step != 1, right? Seems like we should still have this.
There was a problem hiding this comment.
It shouldn't since step is either controlled by the caller (we shouldn't deallocate) or usize_one (we shouldn't deallocate either) the if before was because we were allocating within make_usize_constant
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Changes to circuit sizes
🧾 Summary (100% most significant diffs)
Full diff report 👇
|
4 participants
Changes:
Before => After public function sizes