MB-36029: Shard Timings::interval_counters by core.

Cache contention was observed when many threads attempt to update the same interval_counters elements at the same time. Shard this by core to reduce the contention. False sharing was also observed with Bucket::response_counters. Cacheline pad the Timings object to prevent this. Perf stats (Triton 2 Node 80/20 R/W test): Before: 3,511,611 (Average of 24 runs) After: 3,563,007 (Average of 5 runs) Change-Id: I854bc654072f6789c82296a6e10cb54e8d5cdd13 Reviewed-on: http://review.couchbase.org/111868 Reviewed-by: Dave Rigby <[email protected]> Tested-by: Build Bot <[email protected]>
trondn · Sep 18, 2019 · 3c221fd · 3c221fd
1 parent 0dd246f
commit 3c221fd
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 13 deletions.
diff --git a/daemon/timings.cc b/daemon/timings.cc
@@ -51,8 +51,10 @@ void Timings::collect(cb::mcbp::ClientOpcode opcode,
     get_or_create_timing_histogram(
             std::underlying_type<cb::mcbp::ClientOpcode>::type(opcode))
             .add(duration_cast<microseconds>(nsec));
-    auto& interval = interval_counters
-            [std::underlying_type<cb::mcbp::ClientOpcode>::type(opcode)];
+    auto& interval =
+            interval_counters
+                    .get()[std::underlying_type<cb::mcbp::ClientOpcode>::type(
+                            opcode)];
     interval.count++;
     interval.duration_ns += nsec.count();
 }
@@ -170,19 +172,21 @@ void Timings::sample(std::chrono::seconds sample_interval) {
     cb::sampling::Interval interval_lookup, interval_mutation;
 
     for (auto op : timings_mutations) {
-        interval_mutation += interval_counters
-                [std::underlying_type<cb::mcbp::ClientOpcode>::type(op)];
-        interval_counters[std::underlying_type<cb::mcbp::ClientOpcode>::type(
-                                  op)]
-                .reset();
+        for (auto intervals : interval_counters) {
+            interval_mutation += intervals
+                    [std::underlying_type<cb::mcbp::ClientOpcode>::type(op)];
+            intervals[std::underlying_type<cb::mcbp::ClientOpcode>::type(op)]
+                    .reset();
+        }
     }
 
     for (auto op : timings_retrievals) {
-        interval_lookup += interval_counters
-                [std::underlying_type<cb::mcbp::ClientOpcode>::type(op)];
-        interval_counters[std::underlying_type<cb::mcbp::ClientOpcode>::type(
-                                  op)]
-                .reset();
+        for (auto intervals : interval_counters) {
+            interval_mutation += intervals
+                    [std::underlying_type<cb::mcbp::ClientOpcode>::type(op)];
+            intervals[std::underlying_type<cb::mcbp::ClientOpcode>::type(op)]
+                    .reset();
+        }
     }
 
     {

diff --git a/daemon/timings.h b/daemon/timings.h
@@ -20,6 +20,7 @@
 
 #include <mcbp/protocol/opcode.h>
 
+#include <platform/corestore.h>
 #include <utilities/hdrhistogram.h>
 #include <array>
 #include <mutex>
@@ -72,5 +73,9 @@ class Timings {
     // histogram class
     std::array<std::atomic<Hdr1sfMicroSecHistogram*>, MAX_NUM_OPCODES> timings;
     std::mutex histogram_mutex;
-    std::array<cb::sampling::Interval, MAX_NUM_OPCODES> interval_counters;
+
+    // Sharded by core as cache contention was observed due to the number of
+    // threads attempting to update the same timings stats.
+    CoreStore<std::array<cb::sampling::Interval, MAX_NUM_OPCODES>>
+            interval_counters;
 };