Remove pointless mutex

Author: graydon

src/ledger/SorobanMetrics.cpp

@@ -250,7 +250,6 @@ SorobanMetrics::flushApplyMetricsBatches()
     };
     for (auto const& b : batches)
     {
-        std::lock_guard<std::mutex> lock(b->mMutex);
         total.mHostFnOpReadEntry += take(b->mHostFnOpReadEntry);
         total.mHostFnOpWriteEntry += take(b->mHostFnOpWriteEntry);
         total.mHostFnOpReadKeyByte += take(b->mHostFnOpReadKeyByte);

src/ledger/SorobanMetrics.h

@@ -32,14 +32,16 @@ class SorobanMetrics
 {
   public:
     // Accumulates apply-path metric updates from a single thread. Hot apply
-    // code records into its own thread's batch (brief, uncontended lock
-    // acquisition), and the batches are drained into the underlying
-    // process-wide medida metrics once per ledger on the main thread via
-    // publishAndResetLedgerWideMetrics(). This keeps shared metric state (its
-    // locks and cache lines) off the parallel apply threads.
+    // code records into its own thread's batch, and the batches are drained
+    // into the underlying process-wide medida metrics once per ledger on the
+    // main thread via publishAndResetLedgerWideMetrics(). This both avoids fine
+    // grained locking and leverages batch update interfaces on medida.
+    //
+    // Note: this class is _not_ threadsafe. An instance is owned by each thread
+    // ( in a map keyed by thread ID) and each thread should only access its
+    // own.
     struct ApplyMetricsBatch
     {
-        std::mutex mMutex;
 
         // Pending Meter increments (Marks summed since the last publish).
         uint64_t mHostFnOpReadEntry{0};
@@ -268,7 +270,6 @@ class BatchedTimerScope
                            std::chrono::steady_clock::now() - mStart)
                            .count();
         auto& batch = mMetrics.getApplyThreadBatch();
-        std::lock_guard<std::mutex> lock(batch.mMutex);
         (batch.*mField).push_back(elapsed);
     }
 

src/transactions/ExtendFootprintTTLOpFrame.cpp

@@ -44,7 +44,6 @@ struct ExtendFootprintTTLMetrics
         // rather than updating process-wide metrics from every (possibly
         // concurrent) operation.
         auto& batch = mMetrics.getApplyThreadBatch();
-        std::lock_guard<std::mutex> lock(batch.mMutex);
         batch.mExtFpTtlOpReadLedgerByte += mLedgerReadByte;
         if (mExecTimed)
         {

src/transactions/InvokeHostFunctionOpFrame.cpp

@@ -166,7 +166,6 @@ struct HostFunctionMetrics
         // per ledger) instead of updating ~25 process-wide metrics from
         // every (possibly concurrent) operation.
         auto& batch = mMetrics.getApplyThreadBatch();
-        std::lock_guard<std::mutex> lock(batch.mMutex);
 
         batch.mHostFnOpReadEntry += mReadEntry;
         batch.mHostFnOpWriteEntry += mWriteEntry;

src/transactions/RestoreFootprintOpFrame.cpp

@@ -46,7 +46,6 @@ struct RestoreFootprintMetrics
         // rather than updating process-wide metrics from every (possibly
         // concurrent) operation.
         auto& batch = mMetrics.getApplyThreadBatch();
-        std::lock_guard<std::mutex> lock(batch.mMutex);
         batch.mRestoreFpOpReadLedgerByte += mLedgerReadByte;
         batch.mRestoreFpOpWriteLedgerByte += mLedgerWriteByte;
         if (mExecTimed)

src/transactions/TransactionFrame.cpp

@@ -1099,7 +1099,6 @@ TransactionFrame::updateSorobanMetrics(AppConnector& app) const
     // ledger)
     {
         auto& batch = metrics.getApplyThreadBatch();
-        std::lock_guard<std::mutex> lock(batch.mMutex);
         batch.mTxSizeByte.push_back(txSize);
     }
     // accumulate the ledger-wide metrics, which will get emitted at the ledger