[DO_NOT_MERGE] feat: optimizing private_set.nr
#7798
Closed
Conversation
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 stack of pull requests is managed by Graphite. Learn more about stacking. |
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 | | |
benesjan
force-pushed
the
08-06-feat_optimizing_private_set.nr
branch
from
d53fe8c to
34acf74
Compare
benesjan
changed the title
private_set.nr
benesjan
commented
Aug 7, 2024
| @@ -120,42 +121,46 @@ impl Deck<&mut PrivateContext> { | |||
| inserted_cards | |||
| } | |||
|
|
|||
| pub fn get_cards<N>(&mut self, cards: [Card; N]) -> [CardNote; N] { | |||
| pub fn get_cards_and_note_hashes<N>(&mut self, cards: [Card; N]) -> ([CardNote; N], [Field; N]) { | |||
There was a problem hiding this comment.
This function became a bit ugly with the notes and hashes because it didn't work well with the custom CardNote struct (made it difficult to couple the note and hash in the NoteAndHash struct). But I decided against doing a larger refactor since the ugliness is mostly just in this contract.
benesjan
force-pushed
the
08-06-feat_optimizing_private_set.nr
branch
from
e399ddf to
5804783
Compare
benesjan
force-pushed
the
08-06-feat_optimizing_private_set.nr
branch
from
5804783 to
b79e4e4
Compare
benesjan
changed the title
private_set.nrprivate_set.nr
|
Closed in favor of #7834 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We unnecessarily compute note_hash_for_read_request twice when removing a note: once when getting it via get_notes and the second time in PrivateSet::remove. This is expensive. This PRs removes the redundant hash computation by passing the hashes from get_notes.
Gate count change of Token transfer:
Before 93k gates.
After:
Gate count change: 93k-89k=4k gates savings.
Note for reviewer
I initially had 2 get notes functions: get_notes and get_notes_and_hashes. As the name implies get_notes returned "pure notes" and get_notes_and_hashes returned both the note and the hash. When I checked usage of the get_notes method it was only used in test contracts and hence would not consider them a real use case. So it generally seems that whenever we obtain a note we will most likely also want to destroy it. Hence I merged the 2 functions and now there is only 1 get_notes func.