-
Notifications
You must be signed in to change notification settings - Fork 115
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(avm): get_row optimization - 25x faster logderiv inv (#11605)
Proving times (VM1) on 16 cores, 850+ columns, dozens of lookups, bulk_test. ``` ** Before ** prove/all_ms: 92606 prove/execute_log_derivative_inverse_round_ms: 21544 ** After ** prove/all_ms: 73404 prove/execute_log_derivative_inverse_round_ms: 839 ``` No change in sumcheck time. For reviewing, you can focus on the templates. An explanation follows (with history). --- This PR is about the `get_row()` method on the prover polynomials of a given flavor. This method is used by the [logderivative library](https://github.com/AztecProtocol/aztec-packages/blob/master/barretenberg/cpp/src/barretenberg/honk/proof_system/logderivative_library.hpp#L36) to compute logderivative inverses. Originally, `get_row()` was supposed to be debug only but it ended up used in the library above. To be fair, the reason is as follows: the `accumulate` function of relations (including lookups and perms), takes in a row (or something that looks like it!). However, by the time that you have to compute inverses, you don't have your row-based trace anymore, you only have the prover polynomials which are column-based. So, you need to extract a row from columns. The following sections explore a way to make things run faster, without completely breaking the `get_row()` expectations from the caller. That is, that it behaves like a row (you can do `.column` and it will return the field for it). # Phase 1: `AllEntities<FF>` So far so good. Normal [BB flavors](https://github.com/AztecProtocol/aztec-packages/blob/master/barretenberg/cpp/src/barretenberg/stdlib_circuit_builders/mega_flavor.hpp#L366) make `get_row()` return `AllEntities<FF>` which is literally a row with as many fields copied as columns you have. Note that the copy is done even for the columns that may not get used later in the accumulation of a relation, or in the computation of inverses. This might be ok if you have 10 columns and a handful of lookups, but in our case we have dozens of lookups and 850+ columns (we estimate 3500 by completion of the AVM). # Phase 2: something like `AllEntities<const FF&>` As a quick fix you might think you can copy references instead and use `AllEntities<const FF&>`. Well you can't, at least not the way you would use `AllEntities<FF>`. Since the class would have members that are references, you need to define a constructor that initializes them all, maybe from a `RefArray` of sorts. The problem is because the class `AllEntities` is defined as inheriting from other classes, instead of being "flat". This, for us, added an immense amount of codegen. See `AllConstRefValues` [here](https://github.com/AztecProtocol/aztec-packages/blob/2f05dc02fe7b147c7cd6fc235134279dbf332c08/barretenberg/cpp/src/barretenberg/vm/avm/generated/flavor.cpp). This improvement was introduced in [this PR](AztecProtocol/aztec-packages#7419) and it gave a **20x** speed improvement over `AllEntities<FF>`. The code itself was then improved in [this PR](AztecProtocol/aztec-packages#11504) by using a flat class and some fold expressions. # Phase 3: Getters Ideally what we'd want is for `get_row()` to return something like this: ``` template <typename Polynomials> class PolynomialEntitiesAtFixedRow { public: PolynomialEntitiesAtFixedRow(const size_t row_idx, const Polynomials& pp) : row_idx(row_idx) , pp(pp) {} // what here? private: const size_t row_idx; const Polynomials& pp; }; ``` such that if you do `row.column` it would secretly do `pp.column[row_idx]` instead. Unfortunately, you cannot override the `.` operator, and certainly not like this. Instead, we compromise. I added a macro to generate getters `_column()` for every column, which do exactly that. Then I changed the lookups and permutation codegen to use that (i.e., `in._column()` instead of `in.column`). Note that we _only_ use these getters in lookups and perm, not in the main relations. However, we are not done. The perms and lookups code that we changed is also called by `accumulate` when doing sumcheck, and `AllEntities` does not provide those getters so it will not compile. Well, we add them, and we are done. This results in a **25x** time improvement in calculating logderiv inverses, amounting to a total of **500x** better than baseline. # Conclusion Some thing in BB are not thought for a VM :) I wonder if theres any such improvement lurking in sumcheck? :)
- Loading branch information
Showing
29 changed files
with
1,409 additions
and
1,336 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.