Cleaned up types in BucketSnapshot

src/bucket/BucketListSnapshot.h

@@ -86,9 +86,8 @@ class SearchableBucketListSnapshotBase : public NonMovableOrCopyable
     BucketSnapshotManager const& mSnapshotManager;
 
     // Snapshot managed by SnapshotManager
-    std::unique_ptr<BucketListSnapshot<BucketT> const> mSnapshot{};
-    std::map<uint32_t, std::unique_ptr<BucketListSnapshot<BucketT> const>>
-        mHistoricalSnapshots;
+    SnapshotPtrT<BucketT> mSnapshot{};
+    std::map<uint32_t, SnapshotPtrT<BucketT>> mHistoricalSnapshots;
 
     // Loops through all buckets, starting with curr at level 0, then snap at
     // level 0, etc. Calls f on each bucket. Exits early if function

src/bucket/BucketSnapshotManager.cpp

@@ -5,6 +5,7 @@
 #include "bucket/BucketSnapshotManager.h"
 #include "bucket/Bucket.h"
 #include "bucket/BucketListSnapshot.h"
+#include "bucket/BucketUtils.h"
 #include "main/Application.h"
 #include "util/GlobalChecks.h"
 #include "util/XDRStream.h" // IWYU pragma: keep
@@ -17,10 +18,8 @@ namespace stellar
 {
 
 BucketSnapshotManager::BucketSnapshotManager(
-    Application& app,
-    std::unique_ptr<BucketListSnapshot<LiveBucket> const>&& snapshot,
-    std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>&&
-        hotArchiveSnapshot,
+    Application& app, SnapshotPtrT<LiveBucket>&& snapshot,
+    SnapshotPtrT<HotArchiveBucket>&& hotArchiveSnapshot,
     uint32_t numLiveHistoricalSnapshots)
     : mApp(app)
     , mCurrLiveSnapshot(std::move(snapshot))
@@ -81,42 +80,38 @@ BucketSnapshotManager::recordBulkLoadMetrics(std::string const& label,
     return iter->second;
 }
 
-template <class SnapshotT>
+template <>
 void
-BucketSnapshotManager::maybeUpdateSnapshot(
-    std::unique_ptr<SnapshotT const>& snapshot,
-    std::map<uint32_t, std::unique_ptr<SnapshotT const>>& historicalSnapshots)
-    const
+BucketSnapshotManager::maybeUpdateSnapshot<LiveBucket>(
+    SnapshotPtrT<LiveBucket>& snapshot,
+    std::map<uint32_t, SnapshotPtrT<LiveBucket>>& historicalSnapshots) const
 {
-    static_assert(
-        std::is_same_v<SnapshotT, BucketListSnapshot<LiveBucket>> ||
-        std::is_same_v<SnapshotT, BucketListSnapshot<HotArchiveBucket>>);
+    maybeUpdateSnapshotInternal(snapshot, historicalSnapshots,
+                                mCurrLiveSnapshot, mLiveHistoricalSnapshots);
+}
 
-    auto const& managerSnapshot = [&]() -> auto const&
-    {
-        if constexpr (std::is_same_v<SnapshotT, BucketListSnapshot<LiveBucket>>)
-        {
-            return mCurrLiveSnapshot;
-        }
-        else
-        {
-            return mCurrHotArchiveSnapshot;
-        }
-    }
-    ();
+template <>
+void
+BucketSnapshotManager::maybeUpdateSnapshot<HotArchiveBucket>(
+    SnapshotPtrT<HotArchiveBucket>& snapshot,
+    std::map<uint32_t, SnapshotPtrT<HotArchiveBucket>>& historicalSnapshots)
+    const
+{
+    maybeUpdateSnapshotInternal(snapshot, historicalSnapshots,
+                                mCurrHotArchiveSnapshot,
+                                mHotArchiveHistoricalSnapshots);
+}
 
-    auto const& managerHistoricalSnapshots = [&]() -> auto const&
-    {
-        if constexpr (std::is_same_v<SnapshotT, BucketListSnapshot<LiveBucket>>)
-        {
-            return mLiveHistoricalSnapshots;
-        }
-        else
-        {
-            return mHotArchiveHistoricalSnapshots;
-        }
-    }
-    ();
+template <class BucketT>
+void
+BucketSnapshotManager::maybeUpdateSnapshotInternal(
+    SnapshotPtrT<BucketT>& snapshot,
+    std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots,
+    SnapshotPtrT<BucketT> const& managerSnapshot,
+    std::map<uint32_t, SnapshotPtrT<BucketT>> const& managerHistoricalSnapshots)
+    const
+{
+    BUCKET_TYPE_ASSERT(BucketT);
 
     // The canonical snapshot held by the BucketSnapshotManager is not being
     // modified. Rather, a thread is checking it's copy against the canonical
@@ -130,7 +125,8 @@ BucketSnapshotManager::maybeUpdateSnapshot(
         // Should only update with a newer snapshot
         releaseAssert(!snapshot || snapshot->getLedgerSeq() <
                                        managerSnapshot->getLedgerSeq());
-        snapshot = std::make_unique<SnapshotT>(*managerSnapshot);
+        snapshot = std::make_unique<BucketListSnapshot<BucketT> const>(
+            *managerSnapshot);
     }
 
     // Then update historical snapshots (if any exist)
@@ -149,17 +145,17 @@ BucketSnapshotManager::maybeUpdateSnapshot(
         historicalSnapshots.clear();
         for (auto const& [ledgerSeq, snap] : managerHistoricalSnapshots)
         {
-            historicalSnapshots.emplace(ledgerSeq,
-                                        std::make_unique<SnapshotT>(*snap));
+            historicalSnapshots.emplace(
+                ledgerSeq,
+                std::make_unique<BucketListSnapshot<BucketT> const>(*snap));
         }
     }
 }
 
 void
 BucketSnapshotManager::updateCurrentSnapshot(
-    std::unique_ptr<BucketListSnapshot<LiveBucket> const>&& liveSnapshot,
-    std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>&&
-        hotArchiveSnapshot)
+    SnapshotPtrT<LiveBucket>&& liveSnapshot,
+    SnapshotPtrT<HotArchiveBucket>&& hotArchiveSnapshot)
 {
     releaseAssert(threadIsMain());
 
@@ -229,16 +225,4 @@ BucketSnapshotManager::endPointLoadTimer(LedgerEntryType t,
         iter->second.Update(duration);
     }
 }
-
-template void
-BucketSnapshotManager::maybeUpdateSnapshot<BucketListSnapshot<LiveBucket>>(
-    std::unique_ptr<BucketListSnapshot<LiveBucket> const>& snapshot,
-    std::map<uint32_t, std::unique_ptr<BucketListSnapshot<LiveBucket> const>>&
-        historicalSnapshots) const;
-template void BucketSnapshotManager::maybeUpdateSnapshot<
-    BucketListSnapshot<HotArchiveBucket>>(
-    std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>& snapshot,
-    std::map<uint32_t,
-             std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>>&
-        historicalSnapshots) const;
 }

src/bucket/BucketSnapshotManager.h

@@ -30,6 +30,9 @@ template <class BucketT> class BucketListSnapshot;
 class SearchableLiveBucketListSnapshot;
 class SearchableHotArchiveBucketListSnapshot;
 
+template <class BucketT>
+using SnapshotPtrT = std::unique_ptr<BucketListSnapshot<BucketT> const>;
+
 // This class serves as the boundary between non-threadsafe singleton classes
 // (BucketManager, BucketList, Metrics, etc) and threadsafe, parallel BucketList
 // snapshots.
@@ -41,15 +44,12 @@ class BucketSnapshotManager : NonMovableOrCopyable
     // Snapshot that is maintained and periodically updated by BucketManager on
     // the main thread. When background threads need to generate or refresh a
     // snapshot, they will copy this snapshot.
-    std::unique_ptr<BucketListSnapshot<LiveBucket> const> mCurrLiveSnapshot{};
-    std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>
-        mCurrHotArchiveSnapshot{};
+    SnapshotPtrT<LiveBucket> mCurrLiveSnapshot{};
+    SnapshotPtrT<HotArchiveBucket> mCurrHotArchiveSnapshot{};
 
     // ledgerSeq that the snapshot is based on -> snapshot
-    std::map<uint32_t, std::unique_ptr<BucketListSnapshot<LiveBucket> const>>
-        mLiveHistoricalSnapshots;
-    std::map<uint32_t,
-             std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>>
+    std::map<uint32_t, SnapshotPtrT<LiveBucket>> mLiveHistoricalSnapshots;
+    std::map<uint32_t, SnapshotPtrT<HotArchiveBucket>>
         mHotArchiveHistoricalSnapshots;
 
     uint32_t const mNumHistoricalSnapshots;
@@ -66,23 +66,27 @@ class BucketSnapshotManager : NonMovableOrCopyable
 
     mutable std::optional<VirtualClock::time_point> mTimerStart;
 
+    template <class BucketT>
+    void maybeUpdateSnapshotInternal(
+        SnapshotPtrT<BucketT>& snapshot,
+        std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots,
+        SnapshotPtrT<BucketT> const& managerSnapshot,
+        std::map<uint32_t, SnapshotPtrT<BucketT>> const&
+            managerHistoricalSnapshots) const;
+
   public:
     // Called by main thread to update snapshots whenever the BucketList
     // is updated
-    void updateCurrentSnapshot(
-        std::unique_ptr<BucketListSnapshot<LiveBucket> const>&& liveSnapshot,
-        std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>&&
-            hotArchiveSnapshot);
+    void
+    updateCurrentSnapshot(SnapshotPtrT<LiveBucket>&& liveSnapshot,
+                          SnapshotPtrT<HotArchiveBucket>&& hotArchiveSnapshot);
 
     // numHistoricalLedgers is the number of historical snapshots that the
     // snapshot manager will maintain. If numHistoricalLedgers is 5, snapshots
     // will be capable of querying state from ledger [lcl, lcl - 5].
-    BucketSnapshotManager(
-        Application& app,
-        std::unique_ptr<BucketListSnapshot<LiveBucket> const>&& snapshot,
-        std::unique_ptr<BucketListSnapshot<HotArchiveBucket> const>&&
-            hotArchiveSnapshot,
-        uint32_t numHistoricalLedgers);
+    BucketSnapshotManager(Application& app, SnapshotPtrT<LiveBucket>&& snapshot,
+                          SnapshotPtrT<HotArchiveBucket>&& hotArchiveSnapshot,
+                          uint32_t numHistoricalLedgers);
 
     std::shared_ptr<SearchableLiveBucketListSnapshot>
     copySearchableLiveBucketListSnapshot() const;
@@ -91,11 +95,10 @@ class BucketSnapshotManager : NonMovableOrCopyable
     copySearchableHotArchiveBucketListSnapshot() const;
 
     // Checks if snapshot is out of date and updates it accordingly
-    template <class SnapshotT>
-    void
-    maybeUpdateSnapshot(std::unique_ptr<SnapshotT const>& snapshot,
-                        std::map<uint32_t, std::unique_ptr<SnapshotT const>>&
-                            historicalSnapshots) const;
+    template <class BucketT>
+    void maybeUpdateSnapshot(
+        SnapshotPtrT<BucketT>& snapshot,
+        std::map<uint32_t, SnapshotPtrT<BucketT>>& historicalSnapshots) const;
 
     // All metric recording functions must only be called by the main thread
     void startPointLoadTimer() const;