perf/store: optimize updating existing accounts #1567
perf/store: optimize updating existing accounts #1567Mirko-von-Leipzig merged 28 commits intonextfrom
Conversation
drahnr
force-pushed
the
bernhard-do-not-load-full-account
branch
3 times, most recently
from
14dc3a6 to
5f11a29
Compare
There was a problem hiding this comment.
Pull request overview
This PR optimizes applying partial account deltas by avoiding loading full Account state (code bytes, all storage-map entries, all vault assets) during upsert_accounts, instead fetching only minimal state and computing updated headers/roots.
Changes:
- Added an optimized delta-update module to query minimal account state and compute updated
storage_header/vault_root. - Updated
upsert_accountsto use anAccountStateForInsertenum to handle private, full-state, and partial-state inserts. - Added new tests covering the optimized delta path, private upserts, and full-state delta upserts.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/store/src/db/models/queries/accounts/delta.rs | New optimized queries + helpers for partial delta application (minimal state + root/header computation). |
| crates/store/src/db/models/queries/accounts/delta/tests.rs | New tests for optimized partial delta update, private account upsert, and full-state delta upsert. |
| crates/store/src/db/models/queries/accounts.rs | Integrates optimized partial-delta path into upsert_accounts; adds AccountRowInsert constructors. |
| alt_approach.txt | Documents an alternative design (precompute roots in InnerForest). |
| CHANGELOG.md | Adds a changelog entry for the performance improvement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
drahnr
force-pushed
the
bernhard-do-not-load-full-account
branch
from
6b1b40a to
a865ef0
Compare
drahnr
force-pushed
the
bernhard-do-not-load-full-account
branch
from
a865ef0 to
2c632b5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
drahnr
force-pushed
the
bernhard-do-not-load-full-account
branch
from
0a9e28b to
faaad63
Compare
drahnr
force-pushed
the
bernhard-do-not-load-full-account
branch
from
4514d0b to
0b72a94
Compare
| Ok((header, entries)) | ||
| } | ||
|
|
||
| // TODO this is expensive and should only be called from tests |
There was a problem hiding this comment.
Do we want to complete this TODO in this PR?
There was a problem hiding this comment.
Definitely not :)
It's used in State::load to initialize the SmtForest where we still do load the full account from DB, which is the only call-site for this one.
| ); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
There are a lot of diffs here and its hard to get my head around functions like this which have no comments in the body. Could you please break this function into logical sections and describe what is going on in each section with a comment above? 🙏
There was a problem hiding this comment.
(I'm assuming this is a new addition rather than a piece of code moved from elsewhere)
There was a problem hiding this comment.
Added some, a lot of code got re-partitioned due to the increased complexity, and git doesn't show all diffs correctly.
| /// Optimized query that only fetches balances for the faucet IDs | ||
| /// that are being updated by a delta, rather than loading all vault assets. | ||
| /// | ||
| /// Returns a map from `faucet_id` to the current balance (0 if not found). |
There was a problem hiding this comment.
Is there a use case where the 0 is unacceptably equivocal? i.e. when there is genuinely 0 balance?
There was a problem hiding this comment.
Since we cannot have negative balances in accounts, this would fail, for positive ones it'd initialize with that value. As of today, I don't see issues?
sergerad
left a comment
sergerad
left a comment
There was a problem hiding this comment.
Keen to see the numbers on devnet
Context
Currently applying a partial delta to an existing account requires loading the full account state
consisting of code, all storage map entries, all vault assets, which is very costly - we spend ~250ms on this per block (that's a lot!).
See #1538
What
Optimizes account delta updates by avoiding loading full
Accountobjects from the database when only applying partial deltas.Core changes
module
crates/store/src/db/models/queries/accounts/delta.rsfn select_account_state_for_delta()- fetches only nonce, code_commitment, storage_header, vault_rootfn select_vault_balances_by_faucet_ids()- fetches only specific vault balances being updatedfn apply_storage_delta_with_precomputed_roots()- updates the storage header using precomputed map rootsInnerForest::apply_block_updatesnow precomputes vault and storage map roots for each account update and passes them toDb::apply_blockfor partial delta updates.fn upsert_accountsuses the optimized path for partial deltas viaenum AccountStateForInsertwith three variants:::Private- no public state::FullAccount- new account creation == "full-state delta"::PartialState- incremental update == "partial delta"AccountRowInsertconstructors:new_private(),new_from_account(),new_from_partial()Biggest Change
We reverse the flow and update - we update the
InnerForestnow and track new roots, then using these in the DB storage header update. The downside is additional tackling.