SERVER-29534 Place network and op counters on independent cache lines

2 files changed, 66 insertions, 33 deletions

diff --git a/src/mongo/db/stats/counters.cpp b/src/mongo/db/stats/counters.cpp
index 80e3eb3a1d5..d47d9181700 100644
--- a/src/mongo/db/stats/counters.cpp
+++ b/src/mongo/db/stats/counters.cpp
@@ -134,47 +134,69 @@ BSONObj OpCounters::getObj() const {
     return b.obj();
 }
 
-void NetworkCounter::hitPhysical(long long bytesIn, long long bytesOut) {
-    const int64_t MAX = 1ULL << 60;
+void NetworkCounter::hitPhysicalIn(long long bytes) {
+    static const int64_t MAX = 1ULL << 60;
 
     // don't care about the race as its just a counter
-    bool overflow = _physicalBytesIn.loadRelaxed() > MAX || _physicalBytesOut.loadRelaxed() > MAX;
+    const bool overflow = _physicalBytesIn.loadRelaxed() > MAX;
 
     if (overflow) {
-        _physicalBytesIn.store(bytesIn);
-        _physicalBytesOut.store(bytesOut);
+        _physicalBytesIn.store(bytes);
     } else {
-        _physicalBytesIn.fetchAndAdd(bytesIn);
-        _physicalBytesOut.fetchAndAdd(bytesOut);
+        _physicalBytesIn.fetchAndAdd(bytes);
     }
 }
 
-void NetworkCounter::hitLogical(long long bytesIn, long long bytesOut) {
-    const int64_t MAX = 1ULL << 60;
+void NetworkCounter::hitPhysicalOut(long long bytes) {
+    static const int64_t MAX = 1ULL << 60;
 
     // don't care about the race as its just a counter
-    bool overflow = _logicalBytesIn.loadRelaxed() > MAX || _logicalBytesOut.loadRelaxed() > MAX;
+    const bool overflow = _physicalBytesOut.loadRelaxed() > MAX;
 
     if (overflow) {
-        _logicalBytesIn.store(bytesIn);
-        _logicalBytesOut.store(bytesOut);
+        _physicalBytesOut.store(bytes);
+    } else {
+        _physicalBytesOut.fetchAndAdd(bytes);
+    }
+}
+
+void NetworkCounter::hitLogicalIn(long long bytes) {
+    static const int64_t MAX = 1ULL << 60;
+
+    // don't care about the race as its just a counter
+    const bool overflow = _together.logicalBytesIn.loadRelaxed() > MAX;
+
+    if (overflow) {
+        _together.logicalBytesIn.store(bytes);
         // The requests field only gets incremented here (and not in hitPhysical) because the
         // hitLogical and hitPhysical are each called for each operation. Incrementing it in both
         // functions would double-count the number of operations.
-        _requests.store(1);
+        _together.requests.store(1);
+    } else {
+        _together.logicalBytesIn.fetchAndAdd(bytes);
+        _together.requests.fetchAndAdd(1);
+    }
+}
+
+void NetworkCounter::hitLogicalOut(long long bytes) {
+    static const int64_t MAX = 1ULL << 60;
+
+    // don't care about the race as its just a counter
+    const bool overflow = _logicalBytesOut.loadRelaxed() > MAX;
+
+    if (overflow) {
+        _logicalBytesOut.store(bytes);
     } else {
-        _logicalBytesIn.fetchAndAdd(bytesIn);
-        _logicalBytesOut.fetchAndAdd(bytesOut);
-        _requests.fetchAndAdd(1);
+        _logicalBytesOut.fetchAndAdd(bytes);
     }
 }
 
 void NetworkCounter::append(BSONObjBuilder& b) {
-    b.append("bytesIn", static_cast<long long>(_logicalBytesIn.loadRelaxed()));
+    b.append("bytesIn", static_cast<long long>(_together.logicalBytesIn.loadRelaxed()));
     b.append("bytesOut", static_cast<long long>(_logicalBytesOut.loadRelaxed()));
     b.append("physicalBytesIn", static_cast<long long>(_physicalBytesIn.loadRelaxed()));
     b.append("physicalBytesOut", static_cast<long long>(_physicalBytesOut.loadRelaxed()));
-    b.append("numRequests", static_cast<long long>(_requests.loadRelaxed()));
+    b.append("numRequests", static_cast<long long>(_together.requests.loadRelaxed()));
 }
 
 
diff --git a/src/mongo/db/stats/counters.h b/src/mongo/db/stats/counters.h
index 435542beb37..1522da5b1aa 100644
--- a/src/mongo/db/stats/counters.h
+++ b/src/mongo/db/stats/counters.h
@@ -35,6 +35,7 @@
 #include "mongo/util/concurrency/spin_lock.h"
 #include "mongo/util/net/message.h"
 #include "mongo/util/processinfo.h"
+#include "mongo/util/with_alignment.h"
 
 namespace mongo {
 
@@ -80,14 +81,12 @@ public:
 private:
     void _checkWrap();
 
-    // todo: there will be a lot of cache line contention on these.  need to do something
-    //       else eventually.
-    AtomicUInt32 _insert;
-    AtomicUInt32 _query;
-    AtomicUInt32 _update;
-    AtomicUInt32 _delete;
-    AtomicUInt32 _getmore;
-    AtomicUInt32 _command;
+    CacheAligned<AtomicUInt32> _insert;
+    CacheAligned<AtomicUInt32> _query;
+    CacheAligned<AtomicUInt32> _update;
+    CacheAligned<AtomicUInt32> _delete;
+    CacheAligned<AtomicUInt32> _getmore;
+    CacheAligned<AtomicUInt32> _command;
 };
 
 extern OpCounters globalOpCounters;
@@ -96,19 +95,31 @@ extern OpCounters replOpCounters;
 class NetworkCounter {
 public:
     // Increment the counters for the number of bytes read directly off the wire
-    void hitPhysical(long long bytesIn, long long bytesOut);
+    void hitPhysicalIn(long long bytes);
+    void hitPhysicalOut(long long bytes);
+
     // Increment the counters for the number of bytes passed out of the TransportLayer to the
     // server
-    void hitLogical(long long bytesIn, long long bytesOut);
+    void hitLogicalIn(long long bytes);
+    void hitLogicalOut(long long bytes);
+
     void append(BSONObjBuilder& b);
 
 private:
-    AtomicInt64 _physicalBytesIn{0};
-    AtomicInt64 _physicalBytesOut{0};
-    AtomicInt64 _logicalBytesIn{0};
-    AtomicInt64 _logicalBytesOut{0};
+    CacheAligned<AtomicInt64> _physicalBytesIn{0};
+    CacheAligned<AtomicInt64> _physicalBytesOut{0};
+
+    // These two counters are always incremented at the same time, so
+    // we place them on the same cache line.
+    struct Together {
+        AtomicInt64 logicalBytesIn{0};
+        AtomicInt64 requests{0};
+    };
+    CacheAligned<Together> _together{};
+    static_assert(sizeof(decltype(_together)) <= stdx::hardware_constructive_interference_size,
+                  "cache line spill");
 
-    AtomicInt64 _requests{0};
+    CacheAligned<AtomicInt64> _logicalBytesOut{0};
 };
 
 extern NetworkCounter networkCounter;