perf(nds): store only the nodes a commit changed - #1200
Merged
emturner merged 0 commit intoSep 9, 2026
Conversation
|
Benchmark results for revision 2fc210c:
Full results
Compare the results above with those for the default branch. |
emturner
force-pushed
the
emturner@working-tip
branch
from
September 3, 2026 20:54
e4431d0 to
d42b8ce
Compare
emturner
force-pushed
the
emturner@tzx-213-commit-only-changed-nodes
branch
from
September 3, 2026 20:59
d6444fc to
0775214
Compare
emturner
marked this pull request as draft
September 3, 2026 21:02
|
Durable storage space metrics for revision 2185dc0, at pull request
Full reportThese figures are for catching a regression before it merges. Compare them with |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## emturner@working-tip #1200 +/- ##
========================================================
- Coverage 90.50% 90.46% -0.04%
========================================================
Files 153 153
Lines 32995 33060 +65
Branches 32995 33060 +65
========================================================
+ Hits 29862 29909 +47
- Misses 2136 2151 +15
- Partials 997 1000 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
emturner
force-pushed
the
emturner@tzx-213-commit-only-changed-nodes
branch
from
September 4, 2026 11:37
0775214 to
8d0605e
Compare
emturner
force-pushed
the
emturner@working-tip
branch
from
September 4, 2026 11:37
d42b8ce to
514e264
Compare
emturner
force-pushed
the
emturner@tzx-213-commit-only-changed-nodes
branch
from
September 4, 2026 11:58
8d0605e to
7df79c8
Compare
emturner
force-pushed
the
emturner@working-tip
branch
2 times, most recently
from
September 7, 2026 16:59
3b3b26d to
c927784
Compare
emturner
force-pushed
the
emturner@tzx-213-commit-only-changed-nodes
branch
from
September 7, 2026 16:59
7df79c8 to
7b26618
Compare
emturner
force-pushed
the
emturner@working-tip
branch
from
September 8, 2026 09:06
c927784 to
8fa5eee
Compare
emturner
force-pushed
the
emturner@tzx-213-commit-only-changed-nodes
branch
2 times, most recently
from
September 8, 2026 12:15
7e6c059 to
58b2d8a
Compare
emturner
force-pushed
the
emturner@tzx-213-commit-only-changed-nodes
branch
from
September 9, 2026 09:50
58b2d8a to
ed21753
Compare
emturner
force-pushed
the
emturner@working-tip
branch
from
September 9, 2026 09:50
8fa5eee to
5dc5755
Compare
This file contains hidden or 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
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.
Part of RV-998.
What
Committing stores only the nodes a commit actually changed. One commit; the compaction and bloom
filter tuning that used to sit with it is now its own PR on top of this one, because it helps a
different thing and carries its own risk.
Why
Committing rewrote every node resolved in memory, changed or not: the recursion in
Storable for Nodestopped only at a child that had never been loaded. The bytes matched what wasalready stored, but they are new writes as far as RocksDB is concerned, so each commit flushed them
into fresh files that no earlier checkpoint shares.
The cost is cumulative, which is the key to reading the numbers below. The resolved set grows
monotonically as a state is committed against repeatedly — for a checked-out state it is everything
touched since — so the redundant write volume ramps with commit count.
How
StoreIdidentifies each live store, and aStoredInmemo onNoderecords which one the bodyreached.
Storable for Nodereturns early when the memo names the store being written, skipping thesubtree: nodes are content-addressed, so an unchanged node cannot have a changed descendant.
invalidate_hashclears the memo, so it is dropped by exactly the mutations that change what a nodehashes to.
Loadable for Nodesets it, which is what stops a checked-out tree rewriting what ittouches.
The memo names a store rather than setting a flag because
copy_databasegives the destination acopy of the source's store and the same in-memory nodes, so one node owes a write to both. Its low
bit records whether the value data went too. It sits behind an
AtomicU64becausestoretakes&self.Performance
gc_space, 200k keys, 1000 modified keys per commit, measured on this branch against"emturner@working-tip". Per-commit figures are for commits that did not follow a compaction:Totals over 20 commits: repository 708.2 -> 560.6 MiB, bytes the history pins 703.7 -> 468.5 MiB
(31.0 -> 19.2 MiB per commit), sharing between consecutive checkpoints 92.7% -> 98.0%.
Node writes are identical in both arms: 139206, 6960 per commit, 260 B average. The same nodes
are stored either way; only redundant rewrites of unchanged ones went away. That is the check that
this is not trading correctness for bytes.
This is invisible to the lifecycle benchmark, by construction
database_lifecycleshows no change on any scenario, including the two second-commit ones. Thatis a property of the harness, not of the commit: it re-clones a pristine, fully lazy checkout for
every iteration (
BatchSize::PerIteration) and commits at most twice, so it measures commit 1-2 ofthe table above, where the two arms are identical. On a lazy tree the pre-existing early return
already prunes everything unresolved, and resolving a path in order to modify a key also changes
every node on it — so there is nothing left to skip.
Verified rather than assumed: with
OCTEZ_NDS_ROCKSDB_DISABLE_AUTO_COMPACTION=1, which removes thecompaction contention that dominates those scenarios (second commit 1.411 s -> 567 ms on
working-tip), the node-skip still shows no change. There is no saving being masked.
Reproduce:
Manually Testing
Run here:
cargo nextest run -p octez-riscv-durable-storage(225/225),cargo clippy -p octez-riscv-durable-storage --all-targets --features="rocksdb unstable-test-utils"(clean),scripts/format.sh --all.TMPDIRmust be on a volume with room — the default/tmpis a 1.5 GB tmpfs and the database testsexhaust it, which surfaces as
OpenRocksDbFailed ... No space left on devicerather than a realfailure.
Regressions
No goldenfiles move: hashing and the stored representation are unchanged, only which writes are
skipped.
Tasks for the Author
StoreId::NONEhad no call sites and is now the memo's sentinel.
Default for StoreIdis reached through#[derive(Default)]onInMemoryKeyValueStore, so it stays, with its docs corrected to say so.test_copied_database_commits_shared_nodes_into_its_own_storereplaces a checked-in proptestseed. Verified to bite: making the memo ignore the store id fails it on both backends with
CommitDataMissing.