Shared metric object for bucket list snapshots

SirTyson · SirTyson · commit a8d0c5feec2e · 2026-06-11T16:02:55.000-07:00
diff --git a/src/bucket/BucketListSnapshot.cpp b/src/bucket/BucketListSnapshot.cpp
@@ -53,27 +53,44 @@ BucketListSnapshotData<BucketT>::BucketListSnapshotData(
 {
 }
 
+//
+// BucketSnapshotMetrics
+//
+
+template <class BucketT>
+BucketSnapshotMetrics<BucketT>::BucketSnapshotMetrics(MetricsRegistry& metrics)
+    : mPointTimers([&metrics]() {
+        UnorderedMap<LedgerEntryType, std::reference_wrapper<SimpleTimer>>
+            timers;
+        for (auto t : xdr::xdr_traits<LedgerEntryType>::enum_values())
+        {
+            auto const& label = xdr::xdr_traits<LedgerEntryType>::enum_name(
+                static_cast<LedgerEntryType>(t));
+            auto& metric = metrics.NewSimpleTimer(
+                {BucketT::METRIC_STRING, label}, std::chrono::microseconds{1});
+            timers.emplace(static_cast<LedgerEntryType>(t), metric);
+        }
+        return timers;
+    }())
+    , mBulkLoadMeter(
+          metrics.NewMeter({BucketT::METRIC_STRING, "query", "loads"}, "query"))
+{
+}
+
 //
 // SearchableBucketListSnapshot
 //
 
 template <class BucketT>
 SearchableBucketListSnapshot<BucketT>::SearchableBucketListSnapshot(
     MetricsRegistry& metrics,
+    std::shared_ptr<BucketSnapshotMetrics<BucketT> const> snapshotMetrics,
     std::shared_ptr<BucketListSnapshotData<BucketT> const> data)
     : mData(std::move(data))
     , mMetrics(metrics)
-    , mBulkLoadMeter(
-          metrics.NewMeter({BucketT::METRIC_STRING, "query", "loads"}, "query"))
+    , mSnapshotMetrics(std::move(snapshotMetrics))
 {
-    for (auto t : xdr::xdr_traits<LedgerEntryType>::enum_values())
-    {
-        auto const& label = xdr::xdr_traits<LedgerEntryType>::enum_name(
-            static_cast<LedgerEntryType>(t));
-        auto& metric = metrics.NewSimpleTimer({BucketT::METRIC_STRING, label},
-                                              std::chrono::microseconds{1});
-        mPointTimers.emplace(static_cast<LedgerEntryType>(t), metric);
-    }
+    releaseAssert(mSnapshotMetrics);
 }
 
 template <class BucketT>
@@ -82,9 +99,8 @@ SearchableBucketListSnapshot<BucketT>::SearchableBucketListSnapshot(
     : mData(other.mData)
     // mStreams intentionally left empty — each copy gets its own stream cache
     , mMetrics(other.mMetrics)
-    , mPointTimers(other.mPointTimers)
+    , mSnapshotMetrics(other.mSnapshotMetrics)
     , mBulkTimers(other.mBulkTimers)
-    , mBulkLoadMeter(other.mBulkLoadMeter)
 {
 }
 
@@ -98,12 +114,11 @@ SearchableBucketListSnapshot<BucketT>::operator=(
         mData = other.mData;
         mStreams.clear();
         mMetrics = other.mMetrics;
-        mPointTimers = other.mPointTimers;
+        mSnapshotMetrics = other.mSnapshotMetrics;
         mBulkTimers = other.mBulkTimers;
-        mBulkLoadMeter = other.mBulkLoadMeter;
 #ifdef BUILD_TESTS
         // Reset thread ownership so the copy can be claimed by another thread.
-        mThreadId = std::thread::id{};
+        mThreadId.store(std::thread::id{});
 #endif
     }
     return *this;
@@ -119,14 +134,13 @@ void
 SearchableBucketListSnapshot<BucketT>::threadInvariant() const
 {
 #ifdef BUILD_TESTS
-    auto current = std::this_thread::get_id();
-    if (mThreadId == std::thread::id{})
-    {
-        mThreadId = current;
-    }
-    else
+    auto const current = std::this_thread::get_id();
+    std::thread::id unclaimed{};
+    // Atomically claim ownership on first use, so any concurrent claimant sees
+    // the CAS fail with `unclaimed` set to the owner's id and asserts.
+    if (!mThreadId.compare_exchange_strong(unclaimed, current))
     {
-        releaseAssert(mThreadId == current);
+        releaseAssert(unclaimed == current);
     }
 #endif
 }
@@ -336,8 +350,8 @@ SearchableBucketListSnapshot<BucketT>::load(LedgerKey const& k) const
     releaseAssert(mData);
     threadInvariant();
 
-    auto timerIter = mPointTimers.find(k.type());
-    releaseAssert(timerIter != mPointTimers.end());
+    auto timerIter = mSnapshotMetrics->mPointTimers.find(k.type());
+    releaseAssert(timerIter != mSnapshotMetrics->mPointTimers.end());
     auto timer = timerIter->second.get().TimeScope();
 
     std::shared_ptr<typename BucketT::LoadT const> result{};
@@ -375,7 +389,7 @@ SearchableBucketListSnapshot<BucketT>::getBulkLoadTimer(
     threadInvariant();
     if (numEntries != 0)
     {
-        mBulkLoadMeter.get().Mark(numEntries);
+        mSnapshotMetrics->mBulkLoadMeter.get().Mark(numEntries);
     }
 
     auto iter = mBulkTimers.find(label);
@@ -402,8 +416,10 @@ SearchableBucketListSnapshot<BucketT>::getSnapshotData() const
 
 SearchableLiveBucketListSnapshot::SearchableLiveBucketListSnapshot(
     MetricsRegistry& metrics,
+    std::shared_ptr<BucketSnapshotMetrics<LiveBucket> const> snapshotMetrics,
     std::shared_ptr<BucketListSnapshotData<LiveBucket> const> data)
-    : SearchableBucketListSnapshot<LiveBucket>(metrics, std::move(data))
+    : SearchableBucketListSnapshot<LiveBucket>(
+          metrics, std::move(snapshotMetrics), std::move(data))
 {
 }
 
@@ -843,8 +859,11 @@ SearchableLiveBucketListSnapshot::scanForEvictionInBucket(
 
 SearchableHotArchiveBucketListSnapshot::SearchableHotArchiveBucketListSnapshot(
     MetricsRegistry& metrics,
+    std::shared_ptr<BucketSnapshotMetrics<HotArchiveBucket> const>
+        snapshotMetrics,
     std::shared_ptr<BucketListSnapshotData<HotArchiveBucket> const> data)
-    : SearchableBucketListSnapshot<HotArchiveBucket>(metrics, std::move(data))
+    : SearchableBucketListSnapshot<HotArchiveBucket>(
+          metrics, std::move(snapshotMetrics), std::move(data))
 {
 }
 
@@ -879,6 +898,8 @@ SearchableHotArchiveBucketListSnapshot::scanAllEntries(
 // Explicit template instantiations
 template struct BucketListSnapshotData<LiveBucket>;
 template struct BucketListSnapshotData<HotArchiveBucket>;
+template struct BucketSnapshotMetrics<LiveBucket>;
+template struct BucketSnapshotMetrics<HotArchiveBucket>;
 template class SearchableBucketListSnapshot<LiveBucket>;
 template class SearchableBucketListSnapshot<HotArchiveBucket>;
 
diff --git a/src/bucket/BucketListSnapshot.h b/src/bucket/BucketListSnapshot.h
@@ -14,6 +14,7 @@
 #include "util/XDRStream.h"
 #include "xdr/Stellar-ledger-entries.h"
 
+#include <atomic>
 #include <functional>
 #include <list>
 #include <memory>
@@ -65,6 +66,24 @@ template <class BucketT> struct BucketListSnapshotData
     explicit BucketListSnapshotData(BucketListBase<BucketT> const& bl);
 };
 
+// Pre-resolved metric references for snapshot queries. Resolving a metric
+// takes the global MetricsRegistry lock, so metrics are resolved once and
+// shared across snapshots rather than re-resolved in every
+// SearchableBucketListSnapshot constructor.
+template <class BucketT> struct BucketSnapshotMetrics
+{
+    BUCKET_TYPE_ASSERT(BucketT);
+
+    // Tracks load times for each LedgerEntryType. We use
+    // SimpleTimer since medida Timer overhead is too expensive for point
+    // loads.
+    UnorderedMap<LedgerEntryType, std::reference_wrapper<SimpleTimer>> const
+        mPointTimers;
+    std::reference_wrapper<medida::Meter> const mBulkLoadMeter;
+
+    explicit BucketSnapshotMetrics(MetricsRegistry& metrics);
+};
+
 // SearchableBucketListSnapshot provides BucketList lookup functionality.
 // Each snapshot maintains its own stream cache for file I/O and a pointer to
 // immutable snapshot data (ledger header, list of referenced buckets, etc).
@@ -96,7 +115,7 @@ template <class BucketT> class SearchableBucketListSnapshot
     // Used by threadInvariant() to assert that a single snapshot is not queried
     // concurrently from multiple threads. Reset on copy so each copy can be
     // claimed by a different thread.
-    mutable std::thread::id mThreadId{};
+    mutable std::atomic<std::thread::id> mThreadId{};
 #endif
 
     // Bucket loads are not thread safe and a single snapshot instance should
@@ -106,16 +125,15 @@ template <class BucketT> class SearchableBucketListSnapshot
 
     std::reference_wrapper<MetricsRegistry> mMetrics;
 
-    // Tracks load times for each LedgerEntryType. We use
-    // SimpleTimer since medida Timer overhead is too expensive for point loads.
-    UnorderedMap<LedgerEntryType, std::reference_wrapper<SimpleTimer>>
-        mPointTimers;
+    // Pre-resolved point load timers and bulk load meter, shared across
+    // snapshots (see BucketSnapshotMetrics).
+    std::shared_ptr<BucketSnapshotMetrics<BucketT> const> mSnapshotMetrics;
 
     // Bulk load timers take significantly longer, so the timer overhead is
-    // comparatively negligible.
+    // comparatively negligible. Resolved lazily per label, so kept
+    // per-instance rather than in BucketSnapshotMetrics.
     mutable UnorderedMap<std::string, std::reference_wrapper<medida::Timer>>
         mBulkTimers;
-    std::reference_wrapper<medida::Meter> mBulkLoadMeter;
 
     // Returns (lazily-constructed) file stream for bucket file. Note
     // this might be in some random position left over from a previous read --
@@ -156,6 +174,7 @@ template <class BucketT> class SearchableBucketListSnapshot
 
     SearchableBucketListSnapshot(
         MetricsRegistry& metrics,
+        std::shared_ptr<BucketSnapshotMetrics<BucketT> const> snapshotMetrics,
         std::shared_ptr<BucketListSnapshotData<BucketT> const> data);
 
   public:
@@ -190,6 +209,8 @@ class SearchableLiveBucketListSnapshot
 {
     SearchableLiveBucketListSnapshot(
         MetricsRegistry& metrics,
+        std::shared_ptr<BucketSnapshotMetrics<LiveBucket> const>
+            snapshotMetrics,
         std::shared_ptr<BucketListSnapshotData<LiveBucket> const> data);
 
     Loop scanForEvictionInBucket(
@@ -230,6 +251,8 @@ class SearchableHotArchiveBucketListSnapshot
 {
     SearchableHotArchiveBucketListSnapshot(
         MetricsRegistry& metrics,
+        std::shared_ptr<BucketSnapshotMetrics<HotArchiveBucket> const>
+            snapshotMetrics,
         std::shared_ptr<BucketListSnapshotData<HotArchiveBucket> const> data);
 
   public:
@@ -246,6 +269,8 @@ class SearchableHotArchiveBucketListSnapshot
 
 extern template struct BucketListSnapshotData<LiveBucket>;
 extern template struct BucketListSnapshotData<HotArchiveBucket>;
+extern template struct BucketSnapshotMetrics<LiveBucket>;
+extern template struct BucketSnapshotMetrics<HotArchiveBucket>;
 extern template class SearchableBucketListSnapshot<LiveBucket>;
 extern template class SearchableBucketListSnapshot<HotArchiveBucket>;
 
diff --git a/src/ledger/ImmutableLedgerView.cpp b/src/ledger/ImmutableLedgerView.cpp
@@ -225,12 +225,16 @@ ImmutableLedgerData::checkInvariant() const
 ImmutableLedgerData::ImmutableLedgerData(
     LiveBucketList const& liveBL, HotArchiveBucketList const& hotArchiveBL,
     LedgerHeaderHistoryEntry const& lcl, HistoryArchiveState const& has,
-    std::optional<SorobanNetworkConfig> sorobanConfig)
+    std::optional<SorobanNetworkConfig> sorobanConfig, MetricsRegistry& metrics)
     : mLiveBucketData(
           std::make_shared<BucketListSnapshotData<LiveBucket>>(liveBL))
     , mHotArchiveBucketData(
           std::make_shared<BucketListSnapshotData<HotArchiveBucket>>(
               hotArchiveBL))
+    , mLiveSnapshotMetrics(
+          std::make_shared<BucketSnapshotMetrics<LiveBucket>>(metrics))
+    , mHotArchiveSnapshotMetrics(
+          std::make_shared<BucketSnapshotMetrics<HotArchiveBucket>>(metrics))
     , mSorobanConfig(std::move(sorobanConfig))
     , mLastClosedLedgerHeader(lcl)
     , mLastClosedHistoryArchiveState(has)
@@ -275,19 +279,22 @@ ImmutableLedgerData::createAndMaybeLoadConfig(
         // Bootstrap: build a lightweight temporary state just to load config
         // from the current live bucket list.
         auto tempState = std::make_shared<ImmutableLedgerData>(
-            liveBL, hotArchiveBL, lcl, has, /*sorobanConfig*/ std::nullopt);
+            liveBL, hotArchiveBL, lcl, has, /*sorobanConfig*/ std::nullopt,
+            metrics);
         ImmutableLedgerView tempView(tempState, metrics);
         sorobanConfig = SorobanNetworkConfig::loadFromLedger(tempView);
     }
-    return std::make_shared<ImmutableLedgerData>(liveBL, hotArchiveBL, lcl, has,
-                                                 std::move(sorobanConfig));
+    return std::make_shared<ImmutableLedgerData>(
+        liveBL, hotArchiveBL, lcl, has, std::move(sorobanConfig), metrics);
 }
 
 ImmutableLedgerView::ImmutableLedgerView(ImmutableLedgerDataPtr state,
                                          MetricsRegistry& metrics)
     : mState(state)
-    , mLiveSnapshot(metrics, state->mLiveBucketData)
-    , mHotArchiveSnapshot(metrics, state->mHotArchiveBucketData)
+    , mLiveSnapshot(metrics, state->mLiveSnapshotMetrics,
+                    state->mLiveBucketData)
+    , mHotArchiveSnapshot(metrics, state->mHotArchiveSnapshotMetrics,
+                          state->mHotArchiveBucketData)
     , mMetrics(metrics)
 {
 }
diff --git a/src/ledger/ImmutableLedgerView.h b/src/ledger/ImmutableLedgerView.h
@@ -279,6 +279,15 @@ class ImmutableLedgerData : public NonMovableOrCopyable
     std::shared_ptr<BucketListSnapshotData<HotArchiveBucket> const> const
         mHotArchiveBucketData;
 
+    // Pre-resolved metric references shared by all views over this state.
+    // Resolving metrics takes the global registry lock, so they are resolved
+    // once here instead of in every view construction, which is on the
+    // per-transaction hot path.
+    std::shared_ptr<BucketSnapshotMetrics<LiveBucket> const> const
+        mLiveSnapshotMetrics;
+    std::shared_ptr<BucketSnapshotMetrics<HotArchiveBucket> const> const
+        mHotArchiveSnapshotMetrics;
+
     std::optional<SorobanNetworkConfig const> const mSorobanConfig;
     LedgerHeaderHistoryEntry const mLastClosedLedgerHeader;
     HistoryArchiveState const mLastClosedHistoryArchiveState;
@@ -295,7 +304,8 @@ class ImmutableLedgerData : public NonMovableOrCopyable
                         HotArchiveBucketList const& hotArchiveBL,
                         LedgerHeaderHistoryEntry const& lcl,
                         HistoryArchiveState const& has,
-                        std::optional<SorobanNetworkConfig> sorobanConfig);
+                        std::optional<SorobanNetworkConfig> sorobanConfig,
+                        MetricsRegistry& metrics);
 
     // Factory: constructs a ImmutableLedgerData, auto-loading the
     // SorobanNetworkConfig from the bucket list when the protocol requires it.
diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
@@ -360,7 +360,7 @@ LedgerManagerImpl::LedgerManagerImpl(Application& app)
 
     auto initialState = std::make_shared<ImmutableLedgerData>(
         bm.getLiveBucketList(), bm.getHotArchiveBucketList(), emptyLcl,
-        emptyHas, /*sorobanConfig*/ std::nullopt);
+        emptyHas, /*sorobanConfig*/ std::nullopt, mApp.getMetrics());
 
     mApplyState.setLedgerState(initialState);
     {
@@ -2135,7 +2135,7 @@ LedgerManagerImpl::buildLedgerState(
         // Caller already loaded config (e.g. from LTX during ledger close)
         return std::make_shared<ImmutableLedgerData>(
             bm.getLiveBucketList(), bm.getHotArchiveBucketList(), lcl, has,
-            std::move(sorobanConfig));
+            std::move(sorobanConfig), mApp.getMetrics());
     }
 
     // Auto-load SorobanNetworkConfig from the BucketList

Original file line number	Diff line number	Diff line change
`@@ -360,7 +360,7 @@ LedgerManagerImpl::LedgerManagerImpl(Application& app)`
`360`	`360`
`361`	`361`	`auto initialState = std::make_shared<ImmutableLedgerData>(`
`362`	`362`	`bm.getLiveBucketList(), bm.getHotArchiveBucketList(), emptyLcl,`
`363`		`- emptyHas, /sorobanConfig/ std::nullopt);`
	`363`	`+ emptyHas, /sorobanConfig/ std::nullopt, mApp.getMetrics());`
`364`	`364`
`365`	`365`	`mApplyState.setLedgerState(initialState);`
`366`	`366`	`{`
`@@ -2135,7 +2135,7 @@ LedgerManagerImpl::buildLedgerState(`
`2135`	`2135`	`// Caller already loaded config (e.g. from LTX during ledger close)`
`2136`	`2136`	`return std::make_shared<ImmutableLedgerData>(`
`2137`	`2137`	`bm.getLiveBucketList(), bm.getHotArchiveBucketList(), lcl, has,`
`2138`		`- std::move(sorobanConfig));`
	`2138`	`+ std::move(sorobanConfig), mApp.getMetrics());`
`2139`	`2139`	`}`
`2140`	`2140`
`2141`	`2141`	`// Auto-load SorobanNetworkConfig from the BucketList`