Use loser-tree merge for ConservationOfLumens - #5427
Open
drebelsky wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Generalizes loser-tree bucket scans to support hot archive entries and reduce ConservationOfLumens memory usage.
Changes:
- Adds generic live/hot-archive current-entry scans.
- Migrates the invariant to deduplicated scans.
- Updates callbacks, indexes, and tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/ledger/InMemorySorobanState.cpp |
Adapts scan callbacks. |
src/ledger/ImmutableLedgerView.h |
Exposes generic scans. |
src/ledger/ImmutableLedgerView.cpp |
Delegates snapshot scans. |
src/invariant/ConservationOfLumens.cpp |
Uses loser-tree scans. |
src/bucket/test/BucketIndexTests.cpp |
Updates live scan tests. |
src/bucket/LiveBucket.h |
Moves range API to base. |
src/bucket/LiveBucket.cpp |
Removes redundant implementation. |
src/bucket/HotArchiveBucketIndex.h |
Adds type-range lookup. |
src/bucket/HotArchiveBucketIndex.cpp |
Implements range lookup. |
src/bucket/BucketListSnapshot.h |
Declares generic current-entry scan. |
src/bucket/BucketListSnapshot.cpp |
Generalizes loser-tree merging. |
src/bucket/BucketBase.h |
Defines shared range API. |
src/bucket/BucketBase.cpp |
Implements shared range lookup. |
Suppressed comments (1)
src/bucket/BucketListSnapshot.cpp:726
- The dependent return type also requires
typename; otherwise this member declaration does not compile.
BucketT::EntryT const&
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| { | ||
| BucketEntry mEntry; | ||
| BUCKET_TYPE_ASSERT(BucketT); | ||
| BucketT::EntryT mEntry; |
Contributor
Author
There was a problem hiding this comment.
In C++20 (because of p0634r3) typename isn't needed for these.
Comment on lines
180
to
181
| // Helper function that processes an entry if it hasn't been seen before. | ||
| // Returns true on success, false on error (with error set in errorMsg). |
Comment on lines
+895
to
+899
| static_assert(std::is_same_v<BucketT, HotArchiveBucket>, | ||
| "unexpected bucket type"); | ||
| if (entry.type() == HOT_ARCHIVE_ARCHIVED) | ||
| { | ||
| if (callback(entry.archivedEntry(), key) == Loop::COMPLETE) |
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.
Adapts the loser tree merge from #5252 to support the hot archive bucket list, as well. This lets us use the merge for the conservation of lumens invariant, which lowers the memory usage. On a noble dev watcher, the node no longer runs into memory problems; over the last week, memory usage (on commit 6c2ccfe) settled to 11.8–12 GB (vs 12–14 on a different watcher running 27.0.0 jammy during the same timespan).