Extract entries by reference from LTX.

Currently `getAllEntries` call seals the LTX and unnecessarily copies every entry, which adds up to a few ms if thousands of entries are involved.

With this change we just return the references to all the entries. Since this call is used just once in the 'production' path, and the results are only used in a couple of read-only calls, it shouldn't be hard to maintain the lifetime correctness for this method.

Author: dmkozh

src/bucket/BucketListBase.cpp

@@ -890,7 +890,6 @@ template void BucketListBase<HotArchiveBucket>::addBatchInternal(
 
 template void BucketListBase<LiveBucket>::addBatchInternal(
     Application& app, uint32_t currLedger, uint32_t currLedgerProtocol,
-    std::vector<LedgerEntry> const& initEntries,
-    std::vector<LedgerEntry> const& liveEntries,
-    std::vector<LedgerKey> const& deadEntries);
+    LedgerEntryRefs const& initEntries, LedgerEntryRefs const& liveEntries,
+    LedgerKeyRefs const& deadEntries);
 }

src/bucket/BucketManager.cpp

@@ -1024,9 +1024,9 @@ BucketManager::forgetUnreferencedBuckets(HistoryArchiveState const& has)
 
 void
 BucketManager::addLiveBatch(Application& app, LedgerHeader header,
-                            std::vector<LedgerEntry> const& initEntries,
-                            std::vector<LedgerEntry> const& liveEntries,
-                            std::vector<LedgerKey> const& deadEntries)
+                            LedgerEntryRefs initEntries,
+                            LedgerEntryRefs liveEntries,
+                            LedgerKeyRefs deadEntries)
 {
     ZoneScoped;
 #ifdef BUILD_TESTS

src/bucket/BucketManager.h

@@ -7,6 +7,7 @@
 #include "bucket/BucketMergeMap.h"
 #include "history/HistoryArchive.h"
 #include "ledger/ImmutableLedgerView.h"
+#include "ledger/LedgerEntryRefs.h"
 #include "ledger/NetworkConfig.h"
 #include "main/Config.h"
 #include "util/ThreadAnnotations.h"
@@ -315,9 +316,8 @@ class BucketManager : NonMovableOrCopyable
     // `header` value should be taken from the ledger at which this batch is
     // being added.
     void addLiveBatch(Application& app, LedgerHeader header,
-                      std::vector<LedgerEntry> const& initEntries,
-                      std::vector<LedgerEntry> const& liveEntries,
-                      std::vector<LedgerKey> const& deadEntries);
+                      LedgerEntryRefs initEntries, LedgerEntryRefs liveEntries,
+                      LedgerKeyRefs deadEntries);
     void addHotArchiveBatch(Application& app, LedgerHeader header,
                             std::vector<LedgerEntry> const& archivedEntries,
                             std::vector<LedgerKey> const& restoredEntries);

src/bucket/LiveBucket.cpp

@@ -377,11 +377,24 @@ LiveBucket::getRangeForType(LedgerEntryType type) const
     return getIndex().getRangeForType(type);
 }
 
+#ifdef BUILD_TESTS
 std::vector<BucketEntry>
 LiveBucket::convertToBucketEntry(bool useInit,
                                  std::vector<LedgerEntry> const& initEntries,
                                  std::vector<LedgerEntry> const& liveEntries,
                                  std::vector<LedgerKey> const& deadEntries)
+{
+    auto initRefs = toRefs(initEntries);
+    auto liveRefs = toRefs(liveEntries);
+    auto deadRefs = toRefs(deadEntries);
+    return convertToBucketEntry(useInit, initRefs, liveRefs, deadRefs);
+}
+#endif
+
+std::vector<BucketEntry>
+LiveBucket::convertToBucketEntry(bool useInit, LedgerEntryRefs initEntries,
+                                 LedgerEntryRefs liveEntries,
+                                 LedgerKeyRefs deadEntries)
 {
     ZoneScoped;
     size_t totalSize =
@@ -393,21 +406,21 @@ LiveBucket::convertToBucketEntry(bool useInit,
     std::vector<BucketEntry*> sortedEntries;
     sortedEntries.reserve(totalSize);
 
-    for (auto const& e : initEntries)
+    for (LedgerEntry const& e : initEntries)
     {
         auto& ce = entries.emplace_back();
         ce.type(useInit ? INITENTRY : LIVEENTRY);
         ce.liveEntry() = e;
         sortedEntries.push_back(&ce);
     }
-    for (auto const& e : liveEntries)
+    for (LedgerEntry const& e : liveEntries)
     {
         auto& ce = entries.emplace_back();
         ce.type(LIVEENTRY);
         ce.liveEntry() = e;
         sortedEntries.push_back(&ce);
     }
-    for (auto const& e : deadEntries)
+    for (LedgerKey const& e : deadEntries)
     {
         auto& ce = entries.emplace_back();
         ce.type(DEADENTRY);
@@ -433,12 +446,27 @@ LiveBucket::convertToBucketEntry(bool useInit,
     return bucket;
 }
 
+#ifdef BUILD_TESTS
 std::shared_ptr<LiveBucket>
 LiveBucket::fresh(BucketManager& bucketManager, uint32_t protocolVersion,
                   std::vector<LedgerEntry> const& initEntries,
                   std::vector<LedgerEntry> const& liveEntries,
                   std::vector<LedgerKey> const& deadEntries,
                   bool countMergeEvents, asio::io_context& ctx, bool doFsync)
+{
+    auto initRefs = toRefs(initEntries);
+    auto liveRefs = toRefs(liveEntries);
+    auto deadRefs = toRefs(deadEntries);
+    return fresh(bucketManager, protocolVersion, initRefs, liveRefs, deadRefs,
+                 countMergeEvents, ctx, doFsync);
+}
+#endif
+
+std::shared_ptr<LiveBucket>
+LiveBucket::fresh(BucketManager& bucketManager, uint32_t protocolVersion,
+                  LedgerEntryRefs initEntries, LedgerEntryRefs liveEntries,
+                  LedgerKeyRefs deadEntries, bool countMergeEvents,
+                  asio::io_context& ctx, bool doFsync)
 {
     ZoneScoped;
     // When building fresh buckets after protocol version 10 (i.e. version
@@ -480,10 +508,9 @@ LiveBucket::fresh(BucketManager& bucketManager, uint32_t protocolVersion,
 std::shared_ptr<LiveBucket>
 LiveBucket::freshInMemoryOnly(BucketManager& bucketManager,
                               uint32_t protocolVersion,
-                              std::vector<LedgerEntry> const& initEntries,
-                              std::vector<LedgerEntry> const& liveEntries,
-                              std::vector<LedgerKey> const& deadEntries,
-                              bool countMergeEvents)
+                              LedgerEntryRefs initEntries,
+                              LedgerEntryRefs liveEntries,
+                              LedgerKeyRefs deadEntries, bool countMergeEvents)
 {
     ZoneScoped;
     // When building fresh buckets after protocol version 10 (i.e. version

src/bucket/LiveBucket.h

@@ -7,6 +7,7 @@
 #include "bucket/BucketBase.h"
 #include "bucket/BucketUtils.h"
 #include "bucket/LiveBucketIndex.h"
+#include "ledger/LedgerEntryRefs.h"
 
 namespace medida
 {
@@ -82,11 +83,17 @@ class LiveBucket : public BucketBase<LiveBucket, LiveBucketIndex>,
     static void checkProtocolLegality(BucketEntry const& entry,
                                       uint32_t protocolVersion);
 
+    static std::vector<BucketEntry>
+    convertToBucketEntry(bool useInit, LedgerEntryRefs initEntries,
+                         LedgerEntryRefs liveEntries,
+                         LedgerKeyRefs deadEntries);
+#ifdef BUILD_TESTS
     static std::vector<BucketEntry>
     convertToBucketEntry(bool useInit,
                          std::vector<LedgerEntry> const& initEntries,
                          std::vector<LedgerEntry> const& liveEntries,
                          std::vector<LedgerKey> const& deadEntries);
+#endif
 
     template <typename InputSource>
     static void mergeCasesWithEqualKeys(
@@ -116,22 +123,27 @@ class LiveBucket : public BucketBase<LiveBucket, LiveBucketIndex>,
     // Create a fresh bucket from given vectors of init (created) and live
     // (updated) LedgerEntries, and dead LedgerEntryKeys. The bucket will
     // be sorted, hashed, and adopted in the provided BucketManager.
+    static std::shared_ptr<LiveBucket>
+    fresh(BucketManager& bucketManager, uint32_t protocolVersion,
+          LedgerEntryRefs initEntries, LedgerEntryRefs liveEntries,
+          LedgerKeyRefs deadEntries, bool countMergeEvents,
+          asio::io_context& ctx, bool doFsync);
+#ifdef BUILD_TESTS
     static std::shared_ptr<LiveBucket>
     fresh(BucketManager& bucketManager, uint32_t protocolVersion,
           std::vector<LedgerEntry> const& initEntries,
           std::vector<LedgerEntry> const& liveEntries,
           std::vector<LedgerKey> const& deadEntries, bool countMergeEvents,
           asio::io_context& ctx, bool doFsync);
+#endif
 
     // Create a fresh bucket that exists only in memory, without writing to
     // disk, calculating a hash, or indexing. This should only be used for
     // "level -1" snap buckets that are immediately merged into level 0.
     static std::shared_ptr<LiveBucket>
     freshInMemoryOnly(BucketManager& bucketManager, uint32_t protocolVersion,
-                      std::vector<LedgerEntry> const& initEntries,
-                      std::vector<LedgerEntry> const& liveEntries,
-                      std::vector<LedgerKey> const& deadEntries,
-                      bool countMergeEvents);
+                      LedgerEntryRefs initEntries, LedgerEntryRefs liveEntries,
+                      LedgerKeyRefs deadEntries, bool countMergeEvents);
 
     // Returns true if the given BucketEntry should be dropped in the bottom
     // level bucket (i.e. DEADENTRY)

src/bucket/LiveBucketList.cpp

@@ -14,9 +14,8 @@ namespace stellar
 void
 LiveBucketList::addBatch(Application& app, uint32_t currLedger,
                          uint32_t currLedgerProtocol,
-                         std::vector<LedgerEntry> const& initEntries,
-                         std::vector<LedgerEntry> const& liveEntries,
-                         std::vector<LedgerKey> const& deadEntries)
+                         LedgerEntryRefs initEntries,
+                         LedgerEntryRefs liveEntries, LedgerKeyRefs deadEntries)
 {
     ZoneScoped;
     addBatchInternal(app, currLedger, currLedgerProtocol, initEntries,
@@ -26,6 +25,21 @@ LiveBucketList::addBatch(Application& app, uint32_t currLedger,
     maybeInitializeCaches(app.getConfig());
 }
 
+#ifdef BUILD_TESTS
+void
+LiveBucketList::addBatch(Application& app, uint32_t currLedger,
+                         uint32_t currLedgerProtocol,
+                         std::vector<LedgerEntry> const& initEntries,
+                         std::vector<LedgerEntry> const& liveEntries,
+                         std::vector<LedgerKey> const& deadEntries)
+{
+    auto initRefs = toRefs(initEntries);
+    auto liveRefs = toRefs(liveEntries);
+    auto deadRefs = toRefs(deadEntries);
+    addBatch(app, currLedger, currLedgerProtocol, initRefs, liveRefs, deadRefs);
+}
+#endif
+
 BucketEntryCounters
 LiveBucketList::sumBucketEntryCounters() const
 {

src/bucket/LiveBucketList.h

@@ -45,11 +45,16 @@ class LiveBucketList : public BucketListBase<LiveBucket>
     // should have spilled due to passing through `currLedger`. The `currLedger`
     // and `currProtocolVersion` values should be taken from the ledger at which
     // this batch is being added.
+    void addBatch(Application& app, uint32_t currLedger,
+                  uint32_t currLedgerProtocol, LedgerEntryRefs initEntries,
+                  LedgerEntryRefs liveEntries, LedgerKeyRefs deadEntries);
+#ifdef BUILD_TESTS
     void addBatch(Application& app, uint32_t currLedger,
                   uint32_t currLedgerProtocol,
                   std::vector<LedgerEntry> const& initEntries,
                   std::vector<LedgerEntry> const& liveEntries,
                   std::vector<LedgerKey> const& deadEntries);
+#endif
 
     BucketEntryCounters sumBucketEntryCounters() const;
 

src/bucket/test/BucketTestUtils.cpp

@@ -37,9 +37,9 @@ getAppLedgerVersion(Application::pointer app)
 
 void
 addLiveBatchAndUpdateSnapshot(Application& app, LedgerHeader header,
-                              std::vector<LedgerEntry> const& initEntries,
-                              std::vector<LedgerEntry> const& liveEntries,
-                              std::vector<LedgerKey> const& deadEntries)
+                              LedgerEntryRefs initEntries,
+                              LedgerEntryRefs liveEntries,
+                              LedgerKeyRefs deadEntries)
 {
     auto& liveBl = app.getBucketManager().getLiveBucketList();
     liveBl.addBatch(app, header.ledgerSeq, header.ledgerVersion, initEntries,
@@ -48,6 +48,18 @@ addLiveBatchAndUpdateSnapshot(Application& app, LedgerHeader header,
     app.getLedgerManager().updateCanonicalStateForTesting(header);
 }
 
+void
+addLiveBatchAndUpdateSnapshot(Application& app, LedgerHeader header,
+                              std::vector<LedgerEntry> const& initEntries,
+                              std::vector<LedgerEntry> const& liveEntries,
+                              std::vector<LedgerKey> const& deadEntries)
+{
+    auto initRefs = toRefs(initEntries);
+    auto liveRefs = toRefs(liveEntries);
+    auto deadRefs = toRefs(deadEntries);
+    addLiveBatchAndUpdateSnapshot(app, header, initRefs, liveRefs, deadRefs);
+}
+
 void
 addHotArchiveBatchAndUpdateSnapshot(
     Application& app, LedgerHeader header,
@@ -181,8 +193,8 @@ LedgerManagerForBucketTests::finalizeLedgerTxnChanges(
     if (mUseTestEntries)
     {
         // Seal the ltx but throw its entries away.
-        std::vector<LedgerEntry> init, live;
-        std::vector<LedgerKey> dead;
+        LedgerEntryRefVec init, live;
+        LedgerKeyRefVec dead;
 
         // Any V20 features must be behind initialLedgerVers check, see comment
         // in LedgerManagerImpl::ledgerClosed
@@ -262,7 +274,7 @@ LedgerManagerForBucketTests::finalizeLedgerTxnChanges(
         // Seal the ltx and collect its entries, but don't load Soroban
         // config yet -- test entries (including network config like eviction
         // iterator) are added directly to the BucketList below, not via ltx.
-        ltx.getAllEntries(init, live, dead);
+        ltx.sealAndBorrowAllEntries(init, live, dead);
 
         // Add dead entries from ltx to entries that will be added to BucketList
         // so we can test background eviction properly
@@ -280,7 +292,7 @@ LedgerManagerForBucketTests::finalizeLedgerTxnChanges(
             // When the actual entries have the same key as test entries, we
             // override the actual entries with the test entries (here we
             // just don't add the actual entries if they're already present).
-            for (auto const& liveEntry : live)
+            for (LedgerEntry const& liveEntry : live)
             {
                 if (std::find_if(mTestLiveEntries.begin(),
                                  mTestLiveEntries.end(),
@@ -295,12 +307,15 @@ LedgerManagerForBucketTests::finalizeLedgerTxnChanges(
         }
 
         // Use the testing values.
+        auto testInitRefs = toRefs(mTestInitEntries);
+        auto testLiveRefs = toRefs(mTestLiveEntries);
+        auto testDeadRefs = toRefs(mTestDeadEntries);
         mApplyState.addAnyContractsToModuleCache(lh.ledgerVersion,
-                                                 mTestInitEntries);
+                                                 testInitRefs);
         mApplyState.addAnyContractsToModuleCache(lh.ledgerVersion,
-                                                 mTestLiveEntries);
-        mApp.getBucketManager().addLiveBatch(
-            mApp, lh, mTestInitEntries, mTestLiveEntries, mTestDeadEntries);
+                                                 testLiveRefs);
+        mApp.getBucketManager().addLiveBatch(mApp, lh, testInitRefs,
+                                             testLiveRefs, testDeadRefs);
 
         // Load the final Soroban config AFTER addLiveBatch so that test
         // entries (e.g. stateArchivalSettings with eviction iterator) are
@@ -325,8 +340,7 @@ LedgerManagerForBucketTests::finalizeLedgerTxnChanges(
         }
 
         mApplyState.updateInMemorySorobanState(
-            mTestInitEntries, mTestLiveEntries, mTestDeadEntries, lh,
-            finalSorobanConfig);
+            testInitRefs, testLiveRefs, testDeadRefs, lh, finalSorobanConfig);
 
         mUseTestEntries = false;
         mAlsoAddActualEntries = false;

src/bucket/test/BucketTestUtils.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "ledger/LedgerEntryRefs.h"
 #include "test/TestUtils.h"
 #include "xdr/Stellar-ledger.h"
 
@@ -12,6 +13,10 @@ namespace stellar
 namespace BucketTestUtils
 {
 
+void addLiveBatchAndUpdateSnapshot(Application& app, LedgerHeader header,
+                                   LedgerEntryRefs initEntries,
+                                   LedgerEntryRefs liveEntries,
+                                   LedgerKeyRefs deadEntries);
 void addLiveBatchAndUpdateSnapshot(Application& app, LedgerHeader header,
                                    std::vector<LedgerEntry> const& initEntries,
                                    std::vector<LedgerEntry> const& liveEntries,
@@ -124,5 +129,6 @@ class BucketTestApplication : public TestApplication
         return std::make_unique<LedgerManagerForBucketTests>(*this);
     }
 };
-}
-}
+
+} // namespace BucketTestUtils
+} // namespace stellar

src/invariant/test/BucketListIsConsistentWithDatabaseTests.cpp

@@ -111,10 +111,10 @@ struct BucketListGenerator
             mLiveKeys.insert(LedgerEntryKey(le));
         }
 
-        std::vector<LedgerEntry> initEntries, liveEntries;
-        std::vector<LedgerKey> deadEntries;
+        LedgerEntryRefVec initEntries, liveEntries;
+        LedgerKeyRefVec deadEntries;
         auto header = ltx.loadHeader().current();
-        ltx.getAllEntries(initEntries, liveEntries, deadEntries);
+        ltx.sealAndBorrowAllEntries(initEntries, liveEntries, deadEntries);
         BucketTestUtils::addLiveBatchAndUpdateSnapshot(
             *app, header, initEntries, liveEntries, deadEntries);
         ltx.commit();