Speed up initial in-memory Soroban state population by drebelsky · Pull Request #5252 · stellar/stellar-core

drebelsky · 2026-05-05T18:17:27Z

Related to #4902. Note that since that time, state churn has continued, so population now takes ~70s on a dev watcher. This PR changes the live state calculation from going through the buckets one-by-one using a hash map to a k-way merge among all the buckets. The merge is done using a loser tree, which gives us about half as many comparisons as using a heap. Running on a dev watcher (ledger 62163071) speeds up from ~70s to ~30s.

Time for 3 runs on upstream vs patch

26.0.2-3192.9b5bee752.noble~do~not~use~in~prd~perftests 300

2026-05-05T18:09:57.683 GB4ZO [Perf INFO] Populated in-memory Soroban state in 31.077 sec
2026-05-05T18:09:57.683 GB4ZO [Perf INFO] Startup state load took 32.777 sec (full=true)

2026-05-05T18:10:33.150 GA6IH [Perf INFO] Populated in-memory Soroban state in 30.273 sec
2026-05-05T18:10:33.151 GA6IH [Perf INFO] Startup state load took 31.619 sec (full=true)

2026-05-05T18:11:08.694 GDECF [Perf INFO] Populated in-memory Soroban state in 30.091 sec
2026-05-05T18:11:08.698 GDECF [Perf INFO] Startup state load took 31.317 sec (full=true)

26.0.2-3190.8a71e20af.noble~perftests 300

2026-05-05T18:05:38.067 GDDEI [Perf INFO] Populated in-memory Soroban state in 68.910 sec
2026-05-05T18:05:38.067 GDDEI [Perf INFO] Startup state load took 70.809 sec (full=true)

2026-05-05T18:07:14.023 GDYRH [Perf INFO] Populated in-memory Soroban state in 70.991 sec
2026-05-05T18:07:14.023 GDYRH [Perf INFO] Startup state load took 73.228 sec (full=true)

2026-05-05T18:08:35.801 GA4XC [Perf INFO] Populated in-memory Soroban state in 74.710 sec
2026-05-05T18:08:35.801 GA4XC [Perf INFO] Startup state load took 76.958 sec (full=true)

Doing the k-way merge also has nicer memory scaling characteristics than the current approach: the amount of memory we use scales with the live state + number of buckets, instead of the current approach that scales with churn (e.g., on a local run, peak memory was ~4.10 GB vs ~7.47 GB).

Additionally, the PR disables bucket merges until after the in-memory state is populated.

Copilot

Pull request overview

This PR speeds up startup-time reconstruction of the in-memory Soroban state by changing live-state discovery from per-bucket deduping to a merged scan across all buckets, and by deferring bucket-merge restart until after full state population. It fits into the ledger/bucket startup path that rebuilds Soroban state from the BucketList on node startup.

Changes:

Replace initializeStateFromSnapshot’s per-type bucket scans with a new “current live entries” scan that returns only the latest live version of each key.
Add bucket-snapshot support for k-way merged live-entry scanning, including a new ledger-key comparator used by the loser-tree merge.
Split bucket merge restart out of assumeState and invoke it later in full startup mode.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`src/ledger/LedgerManagerImpl.cpp`	Defers restarting bucket merges until after full Soroban state setup.
`src/ledger/InMemorySorobanState.cpp`	Switches snapshot initialization to current-live scans for Soroban entry types.
`src/ledger/ImmutableLedgerView.h`	Exposes a new current-live scan API on immutable/apply ledger views.
`src/ledger/ImmutableLedgerView.cpp`	Wires the new ledger-view scan API to the live bucket snapshot.
`src/bucket/LedgerCmp.h`	Declares a 3-way comparator for `LedgerKey` ordering.
`src/bucket/LedgerCmp.cpp`	Implements `LedgerKey` comparison logic used by merged scanning.
`src/bucket/BucketManager.h`	Adds an explicit `restartMerges` API.
`src/bucket/BucketManager.cpp`	Refactors merge restart out of `assumeState` into a separate method.
`src/bucket/BucketListSnapshot.h`	Adds snapshot API for scanning only current live entries of a type.
`src/bucket/BucketListSnapshot.cpp`	Implements the loser-tree/k-way merge scan over bucket entry streams.

bboston7

I'm by no means an expert on the bucket subsystem, but the algorithm implementation looks correct to me. I just had a few questions along the way.

bboston7 · 2026-06-01T17:33:37Z

+// Like LedgerEntryIdCmp, but only compares LedgerKeys, and does a 3-way
+// comparison instead of a less-than.
+std::partial_ordering compareLedgerKeys(LedgerKey const& a, LedgerKey const& b);


Why a partial ordering? Does a total ordering not exist for ledger keys?

The compare delegates to the operator <=> from xdrpp (which is important since we do need to match how LedgerEntryIdCmp does the ordering (and since, e.g., the value type in ScVal for CONTRACT_DATA is nested pretty deeply). I'll open a PR in xdrpp to fix it.

bboston7 · 2026-06-01T17:38:31Z

+SearchableLiveBucketListSnapshot::scanForLiveEntriesOfType(
+    LedgerEntryType type,
+    std::function<void(LedgerEntry const&, LedgerKey const&)> callback) const
+{


This should probably have a ZoneScoped

bboston7 · 2026-06-01T17:43:27Z

+    bool first = true;
+    LedgerKey last;
+    while (tree[0] != exhausted)
+    {
+        int index = tree[0];
+        auto& iter = iterators[index];
+        if (auto& key = iter.getKey(); first || key != last)
+        {
+            last = key;
+            auto& entry = iter.getEntry();
+            if (entry.type() == LIVEENTRY || entry.type() == INITENTRY)
+            {
+                callback(entry.liveEntry(), key);
+            }
+        }
+        first = false;
+
+        if (!iter.advance())
+        {
+            tree[index + numIterators] = exhausted;
+        }
+        int winner = tree[index + numIterators];
+
+        int i = (index + numIterators) / 2;
+        while (i > 0)
+        {
+            if (leftWins(tree[i], winner))
+            {
+                std::swap(tree[i], winner);
+            }
+            i /= 2;
+        }
+
+        tree[0] = winner;


Can you please add some comments to this? It's a little hard to figure out what's going on here. This is the actual k-way merge, right?

Agreed, it's still a little opaque.

SirTyson

Thanks for working on this, it looks like a nice change! It does look like we've added a fair bit of complexity to the scanning function, so I'd like to see some unit tests on the new algo and some randomized testing to make sure we've covered all our bases.

SirTyson · 2026-06-10T21:12:41Z

+} // namespace
+
+void
+SearchableLiveBucketListSnapshot::scanForLiveEntriesOfType(


I think this looks generally correct, but it's adding a good amount of complexity, I'd like to see a unit test specifically for this scanning function. Before it was pretty straight forward and indirectly tested, but given the k-way merge I think a more explicit test is warranted. Maybe we can test some of the loser tree edge cases, like a degenerate merge with just 1 bucket, 2 buckets, and some non powers of two. It might also be a good idea to hook this into the randomized bucket testing infra LedgerStateSnapshotTests,cpp or BucketIndexTests.cpp, where we just make sure we hit all the entries properly.

SirTyson · 2026-06-10T21:14:59Z

+
+        // Update tournament up the tree to the root
+        int i = (index + numIterators) / 2;
+        while (i > 0)


Nit: This while could be a for loop, which to me reads a little cleaner.

SirTyson · 2026-06-10T21:15:42Z

+    bool first = true;
+    LedgerKey last;
+    while (tree[0] != exhausted)
+    {
+        int index = tree[0];
+        auto& iter = iterators[index];
+        if (auto& key = iter.getKey(); first || key != last)
+        {
+            last = key;
+            auto& entry = iter.getEntry();
+            if (entry.type() == LIVEENTRY || entry.type() == INITENTRY)
+            {
+                callback(entry.liveEntry(), key);
+            }
+        }
+        first = false;
+
+        if (!iter.advance())
+        {
+            tree[index + numIterators] = exhausted;
+        }
+        int winner = tree[index + numIterators];
+
+        int i = (index + numIterators) / 2;
+        while (i > 0)
+        {
+            if (leftWins(tree[i], winner))
+            {
+                std::swap(tree[i], winner);
+            }
+            i /= 2;
+        }
+
+        tree[0] = winner;


Agreed, it's still a little opaque.

SirTyson · 2026-06-10T21:18:20Z

+        }
+    }
+
+    auto leftWins = [&iterators](int leftIndex, int rightIndex) -> bool {


More comments here would be helpful, this indicates that the smaller, newer version index wins, right?

SirTyson · 2026-06-10T21:21:47Z

-            return deletedKeys.find(lk) == deletedKeys.end();
+        auto contractDataHandler = [this](LedgerEntry const& le,
+                                          LedgerKey const&) {
+            createContractDataEntry(le);


Something I noticed in createContractDataEntry, we call xdr_size on le, which is a full recursive traversal of the SCVal for data types. Can we return this from readOne and just pipe it through? I remember you mentioned that XDR decode was a pretty significant bottleneck, that might be an easy win.

This ends up not being that expensive, I think it's probably not worth the added complexity. Running locally on ledger 63389567 with the comparison having xdr::xdr_size(...) replaced with 100 in contractCodeSizeForRent and updateStateSizeOnEntryUpdate gives these results.

Current 2026-07-14T16:14:07.328 GBYAB [Perf INFO] Populated in-memory Soroban state in 22.051 sec 2026-07-14T16:14:07.328 GBYAB [Perf INFO] Startup state load took 22.820 sec (full=true) 2026-07-14T16:14:32.035 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.577 sec 2026-07-14T16:14:32.036 GBYAB [Perf INFO] Startup state load took 22.302 sec (full=true) 2026-07-14T16:14:56.716 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.793 sec 2026-07-14T16:14:56.716 GBYAB [Perf INFO] Startup state load took 22.507 sec (full=true) w/o xdr::xdr_size 2026-07-14T16:18:42.282 GBYAB [Perf INFO] Populated in-memory Soroban state in 20.934 sec 2026-07-14T16:18:42.283 GBYAB [Perf INFO] Startup state load took 21.667 sec (full=true) 2026-07-14T16:19:06.451 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.362 sec 2026-07-14T16:19:06.451 GBYAB [Perf INFO] Startup state load took 22.069 sec (full=true) 2026-07-14T16:19:30.936 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.556 sec 2026-07-14T16:19:30.936 GBYAB [Perf INFO] Startup state load took 22.293 sec (full=true)

SirTyson · 2026-06-10T21:23:34Z

-            auto lk = LedgerEntryKey(be.liveEntry());
-            releaseAssertOrThrow(lk.type() == expectedType);
-            return deletedKeys.find(lk) == deletedKeys.end();
+        auto contractDataHandler = [this](LedgerEntry const& le,


Is it worth reserving mContractDataEntries and mCotnratCodeEntries here based on getRangeForType for the buckets?

I'm a little reticent to do it based on getRangeForType since this just gives us byte offsets and not number of entries (and since the extra storage we use will be persistent after the load). Nico did mention one idea of storing the current size in storestate, which would at least let reloads be faster.

Update: I ran locally with exact sizing and the speed-up is negligible

diff

diff --git a/src/ledger/InMemorySorobanState.cpp b/src/ledger/InMemorySorobanState.cpp index cf0e1e690..24549d16f 100644 --- a/src/ledger/InMemorySorobanState.cpp +++ b/src/ledger/InMemorySorobanState.cpp @@ -455,6 +455,9 @@ InMemorySorobanState::initializeStateFromSnapshot( auto ledgerVersion = lclHeader.ledgerVersion; if (protocolVersionStartsFrom(ledgerVersion, SOROBAN_PROTOCOL_VERSION)) { + mContractDataEntries.reserve(3362751); + mContractCodeEntries.reserve(1937); + auto sorobanConfig = SorobanNetworkConfig::loadFromLedger(applyView); auto contractDataHandler = [this](LedgerEntry const& le, LedgerKey const&) { @@ -476,6 +479,9 @@ InMemorySorobanState::initializeStateFromSnapshot( applyView.scanCurrentLiveEntriesOfType(TTL, ttlHandler); applyView.scanCurrentLiveEntriesOfType(CONTRACT_CODE, contractCodeHandler); + + releaseAssert(mContractDataEntries.size() == 3362751); + releaseAssert(mContractCodeEntries.size() == 1937); } mLastClosedLedgerSeq = lclHeader.ledgerSeq;

On ledger 63389567

w/o change 2026-07-14T16:14:07.328 GBYAB [Perf INFO] Populated in-memory Soroban state in 22.051 sec 2026-07-14T16:14:07.328 GBYAB [Perf INFO] Startup state load took 22.820 sec (full=true) 2026-07-14T16:14:32.035 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.577 sec 2026-07-14T16:14:32.036 GBYAB [Perf INFO] Startup state load took 22.302 sec (full=true) 2026-07-14T16:14:56.716 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.793 sec 2026-07-14T16:14:56.716 GBYAB [Perf INFO] Startup state load took 22.507 sec (full=true) w/ change 2026-07-14T16:40:19.939 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.125 sec 2026-07-14T16:40:19.939 GBYAB [Perf INFO] Startup state load took 21.884 sec (full=true) 2026-07-14T16:40:43.969 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.151 sec 2026-07-14T16:40:43.969 GBYAB [Perf INFO] Startup state load took 21.888 sec (full=true) 2026-07-14T16:41:07.882 GBYAB [Perf INFO] Populated in-memory Soroban state in 21.039 sec 2026-07-14T16:41:07.882 GBYAB [Perf INFO] Startup state load took 21.757 sec (full=true)

SirTyson · 2026-06-10T21:26:29Z

+    {
+        return mKey;
+    }
+    bool advance();


Nit: Stylistically this is a little weird. Can we either inline the advance or just move the declaration to .h?

- Add ZoneScoped - Add comments

drebelsky · 2026-07-14T00:46:58Z

Build failure was from being on an out-of-date branch, rebasing

SirTyson

I appreciate the added tests! The BucketList is pretty complex, and the test set up is a bit hard to follow, so I'd appreciate some comments. I also think we should unify the comparison functions, as it's too easy to forget to update one of them if we ever add new types. Implementation wise though, looks good! Thanks for investigating my follow up perf ideas, sorry they didn't seem to be worth the squeeze :(

SirTyson · 2026-07-16T20:34:10Z

+    // exceed the number of ledgers buildBucketList closes
+    // (~levelHalf(mLevelsToBuild - 1)), or eviction would start deleting
+    // entries behind the harness maps' backs.
+    static constexpr uint32_t TTL_DURATION = 10'000;


Can't we just make this arbitrarily huge instead of something reachable like 10k and remove the asserts?

SirTyson · 2026-07-16T20:37:00Z

+        }
+    };
+
+    SECTION("soroban only")


Nit: I don't think this has to be a section, just a {} block makes the test faster.

SirTyson · 2026-07-16T20:37:39Z

+            // the callback
+            ImmutableLedgerView ledgerView =
+                test.getApp().getLedgerManager().copyImmutableLedgerView();
+            ledgerView.scanCurrentLiveEntriesOfType(


For completeness here why not call all non-soroban types?

SirTyson · 2026-07-16T20:51:50Z

+    // (including TTLs), maintained across inserts, updates, and deletes.
+    // Unlike mTestEntries, which only tracks a sampled subset of entries, this
+    // tracks every generated batch. Not maintained by
+    // insertSimilarContractDataKeys, and only insertions are tracked for


Hmm this field seems kind of weird. The test is already complicated, and I don't think we should add something called mAllEntries if it's not maintained in some functions or test cases. Can we either make it such that it's actually correct throughout the test suite, or scope it better to our new tests that rely on it?

SirTyson · 2026-07-16T20:56:08Z

+            BucketIndexTest test{cfg};
+            test.buildMultiVersionTest();
+
+            // ACCOUNT is skipped because genesis entries (the root account)


I think I'd rather bootstrap the mAllEntries field with the source account. I don't care about network configs since we don't generate those, but we do generate account load.

SirTyson · 2026-07-16T21:01:11Z

+                [](LedgerEntry const&, LedgerKey const&) { REQUIRE(false); });
+        };
+
+        SECTION("k disjoint buckets")


It's a little hard to follow what bucket list structure you're building in these test cases. Could you maybe add a block comment at the beginning of each section discussing what the structure is and why it's interesting? Then maybe a comment or two as you build up the structure, just so the reader can follow along and check the bucket looks as intended.

SirTyson · 2026-07-16T21:09:18Z

+            REQUIRE(scan(view, OFFER) == expected);
+        }
+
+        SECTION("same bucket at two slots")


This is a good test case! While it works, I noticed we do re-use the same stream object, which seems like a footgun, as intuitively I would expect separate streams when we're comparing separate logical levels of the BucketList. Is there an easy way to harden this?

I think this is a legit bug, it looks like this actually does fail if there's a slightly more interesting repeat bucket:

SECTION("same bucket at non-adjacent slots with shadowing") { /* Put the same tombstone bucket A in both the newest and oldest * slots, with a conflicting live bucket B between them. Each * logical slot must have an independent stream cursor: the newest * A should shadow B for every key, regardless of the older copy of * A. * * A: DEAD(key1), DEAD(key2) * B: LIVE(key2) * A: DEAD(key1), DEAD(key2) */ auto offers = LedgerTestUtils::generateValidUniqueLedgerEntriesWithTypes( {OFFER}, 2, generatedKeys); std::sort(offers.begin(), offers.end(), [](LedgerEntry const& lhs, LedgerEntry const& rhs) { return LedgerEntryIdCmp{}(lhs.data, rhs.data); }); std::vector<LedgerKey> deadKeys; for (auto const& offer : offers) { deadKeys.emplace_back(LedgerEntryKey(offer)); } auto bucketA = makeBucket({}, {}, deadKeys, /*pad=*/false); auto bucketB = makeBucket({}, {offers.back()}, {}, /*pad=*/false); auto view = makeViewFromBuckets({bucketA, bucketB, bucketA}); requireNoCallback(view, OFFER); }

The test case does fail, am I missing something or calling your test helpers incorrectly?

I misunderstood your first comment. Yeah, I agree it is a bit of a footgun. Updated (and added the test).

SirTyson · 2026-07-16T21:13:47Z

+namespace stellar
+{
+std::partial_ordering
+compareLedgerKeys(LedgerKey const& a, LedgerKey const& b)


Having two basically identical compare functions with different implementations seems like a footgun. Can we refactor this to use a templated internal implementation, such that the logic is unified to a single place? I'm worried we could forget to update one of the comparators, and it would be difficult to spot the divergence in tests.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

SirTyson

Changes look good! I do think the file stream issue actually is a correctness bug, let me know if my unit test is missing something but it seems like it fails.

Copilot AI review requested due to automatic review settings May 5, 2026 18:17

Copilot started reviewing on behalf of drebelsky May 5, 2026 18:18 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Comment thread src/bucket/BucketListSnapshot.cpp Outdated

drebelsky requested a review from bboston7 May 11, 2026 20:58

bboston7 reviewed Jun 1, 2026

View reviewed changes

drebelsky requested review from SirTyson and bboston7 June 5, 2026 17:56

SirTyson requested changes Jun 10, 2026

View reviewed changes

drebelsky force-pushed the faster-in-memory-pop branch from 6309ae3 to bf89017 Compare July 14, 2026 00:08

drebelsky added 4 commits July 13, 2026 17:39

Implement loser-tree merging for populating in-memory soroban state

77795d0

Delay restarting merges in startup path

4b95aa0

Ensure that BucketEntryIterator only yields entries of the proper type

52c0177

Address review feedback

229eb10

- Add ZoneScoped - Add comments

Address review feedback

27b88d3

drebelsky force-pushed the faster-in-memory-pop branch from bf89017 to 27b88d3 Compare July 14, 2026 00:55

drebelsky requested a review from SirTyson July 14, 2026 23:52

SirTyson requested changes Jul 16, 2026

View reviewed changes

drebelsky added 2 commits July 21, 2026 19:00

Clean up tests

2f3cd34

Use strong ordering for xdr <=> comparisons

0009bfb

drebelsky requested a review from Copilot July 24, 2026 21:03

Copilot started reviewing on behalf of drebelsky July 24, 2026 21:03 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread src/bucket/test/BucketIndexTests.cpp Outdated

Comment thread src/bucket/test/BucketIndexTests.cpp Outdated

Fix typos

7c6fbc6

drebelsky requested a review from SirTyson July 27, 2026 16:38

SirTyson requested changes Jul 29, 2026

View reviewed changes

Give each bucket iterator its own stream

0254f8a

drebelsky requested a review from SirTyson July 29, 2026 18:21

Uh oh!

Conversation

drebelsky commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

bboston7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SirTyson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drebelsky commented Jul 14, 2026

Uh oh!

SirTyson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SirTyson Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

drebelsky commented May 5, 2026 •

edited

Loading

SirTyson Jul 16, 2026 •

edited

Loading