better caching

motivation: better metrics cache, seperate mutex per type

changes:
* seperate mutex per metric type
* more tests

Sources/CoreMetrics/Metrics.swift

@@ -120,10 +120,9 @@ public enum Metrics {
 
 private final class CachingMetricsHandler: MetricsHandler {
     private let wrapped: MetricsHandler
-    private let lock = Lock() // TODO: consider lock per cache?
+    private var counters = Cache<Counter>()
-    private var counters = [String: Counter]()
+    private var recorders = Cache<Recorder>()
-    private var Recorders = [String: Recorder]()
+    private var timers = Cache<Timer>()
-    private var timers = [String: Timer]()
 
     public static func wrap(_ handler: MetricsHandler) -> CachingMetricsHandler {
         if let caching = handler as? CachingMetricsHandler {
@@ -138,35 +137,43 @@ private final class CachingMetricsHandler: MetricsHandler {
     }
 
     public func makeCounter(label: String, dimensions: [(String, String)]) -> Counter {
-        return self.make(label: label, dimensions: dimensions, cache: &self.counters, maker: self.wrapped.makeCounter)
+        return counters.getOrSet(label: label, dimensions:dimensions, maker: self.wrapped.makeCounter)
     }
 
     public func makeRecorder(label: String, dimensions: [(String, String)], aggregate: Bool) -> Recorder {
         let maker = { (label: String, dimensions: [(String, String)]) -> Recorder in
             self.wrapped.makeRecorder(label: label, dimensions: dimensions, aggregate: aggregate)
         }
-        return self.make(label: label, dimensions: dimensions, cache: &self.Recorders, maker: maker)
+        return recorders.getOrSet(label: label, dimensions:dimensions, maker: maker)
     }
 
     public func makeTimer(label: String, dimensions: [(String, String)]) -> Timer {
-        return self.make(label: label, dimensions: dimensions, cache: &self.timers, maker: self.wrapped.makeTimer)
+        return timers.getOrSet(label: label, dimensions:dimensions, maker: self.wrapped.makeTimer)
     }
 
-    private func make<Item>(label: String, dimensions: [(String, String)], cache: inout [String: Item], maker: (String, [(String, String)]) -> Item) -> Item {
+    private class Cache<T> {
-        let fqn = self.fqn(label: label, dimensions: dimensions)
+        private var items = [String: T]()
+        // using a mutex is never ideal, we will need to explore optimization options
+        // once we see how real life workloads behaves
+        // for example, for short opetations like hashmap lookup mutexes are worst than r/w locks in 99% reads, but better than them in mixed r/w mode
+        private let lock = Lock()
+        
+        func getOrSet(label: String, dimensions: [(String, String)], maker: (String, [(String, String)]) -> T) -> T {
+            let key = self.fqn(label: label, dimensions: dimensions)            
             return self.lock.withLock {
-            if let item = cache[fqn] {
+                if let item = items[key] {
                     return item
                 } else {
                     let item = maker(label, dimensions)
-                cache[fqn] = item
+                    items[key] = item
                     return item
                 }
             }
         }
         
         private func fqn(label: String, dimensions: [(String, String)]) -> String {
-        return [[label], dimensions.compactMap { $0.1 }].flatMap { $0 }.joined(separator: ".")
+            return [[label], dimensions.compactMap { "\($0.0).\($0.1)" }].flatMap { $0 }.joined(separator: ".")
+        }
     }
 }
 

Tests/MetricsTests/CoreMetricsTests.swift

@@ -194,13 +194,33 @@ class MetricsTests: XCTestCase {
             counter.increment(value)
         }
         handlers.forEach { handler in
-            let counter = handler.counters[name] as! TestCounter
+            let counter = handler.counters[0] as! TestCounter
+            XCTAssertEqual(counter.label, name, "expected label to match")
             XCTAssertEqual(counter.values.count, 1, "expected number of entries to match")
             XCTAssertEqual(counter.values[0].1, Int64(value), "expected value to match")
         }
     }
 
-    func testDimensions() throws {
+    func testCaching() throws {
+        // bootstrap with our test metrics
+        let metrics = TestMetrics()
+        Metrics.bootstrap(metrics)
+        // run the test
+        let name = "counter-\(NSUUID().uuidString)"
+        let counter = Metrics.global.makeCounter(label: name) as! TestCounter
+        // same
+        let name2 = name
+        let counter2 = Metrics.global.makeCounter(label: name2) as! TestCounter
+        XCTAssertEqual(counter2.label, name2, "expected label to match")
+        XCTAssertEqual(counter2, counter, "expected caching to work with dimensions")
+        // different name
+        let name3 = "counter-\(NSUUID().uuidString)"
+        let counter3 = Metrics.global.makeCounter(label: name3) as! TestCounter
+        XCTAssertEqual(counter3.label, name3, "expected label to match")
+        XCTAssertNotEqual(counter3, counter, "expected caching to work with dimensions")
+    }
+    
+    func testCachingWithDimensions() throws {
         // bootstrap with our test metrics
         let metrics = TestMetrics()
         Metrics.bootstrap(metrics)
@@ -208,13 +228,35 @@ class MetricsTests: XCTestCase {
         let name = "counter-\(NSUUID().uuidString)"
         let dimensions = [("foo", "bar"), ("baz", "quk")]
         let counter = Metrics.global.makeCounter(label: name, dimensions: dimensions) as! TestCounter
-        counter.increment()
+        XCTAssertEqual(counter.label, name, "expected dimensions to match")
-
-        XCTAssertEqual(counter.values.count, 1, "expected number of entries to match")
-        XCTAssertEqual(counter.values[0].1, 1, "expected value to match")
         XCTAssertEqual(counter.dimensions.description, dimensions.description, "expected dimensions to match")
-
+        // same
-        let counter2 = Metrics.global.makeCounter(label: name, dimensions: dimensions) as! TestCounter
+        let name2 = name
+        let dimensions2 = dimensions
+        let counter2 = Metrics.global.makeCounter(label: name2, dimensions: dimensions2) as! TestCounter
+        XCTAssertEqual(counter2.label, name2, "expected label to match")
+        XCTAssertEqual(counter2.dimensions.description, dimensions2.description, "expected dimensions to match")
         XCTAssertEqual(counter2, counter, "expected caching to work with dimensions")
+        // different name
+        let name3 = "counter-\(NSUUID().uuidString)"
+        let dimensions3 = dimensions
+        let counter3 = Metrics.global.makeCounter(label: name3, dimensions: dimensions3) as! TestCounter
+        XCTAssertEqual(counter3.label, name3, "expected label to match")
+        XCTAssertEqual(counter3.dimensions.description, dimensions3.description, "expected dimensions to match")
+        XCTAssertNotEqual(counter3, counter, "expected caching to work with dimensions")
+        // different dimensions "key"
+        let name4 = name
+        let dimensions4 = dimensions.map{ ($0.0 + "-test" , $0.1) }
+        let counter4 = Metrics.global.makeCounter(label: name4, dimensions: dimensions4) as! TestCounter
+        XCTAssertEqual(counter4.label, name4, "expected label to match")
+        XCTAssertEqual(counter4.dimensions.description, dimensions4.description, "expected dimensions to match")
+        XCTAssertNotEqual(counter4, counter, "expected caching to work with dimensions")
+        // different dimensions "value"
+        let name5 = name
+        let dimensions5 = dimensions.map{ ($0.0, $0.1 + "-test") }
+        let counter5 = Metrics.global.makeCounter(label: name5, dimensions: dimensions5) as! TestCounter
+        XCTAssertEqual(counter5.label, name5, "expected label to match")
+        XCTAssertEqual(counter5.dimensions.description, dimensions5.description, "expected dimensions to match")
+        XCTAssertNotEqual(counter5, counter, "expected caching to work with dimensions")
     }
 }

Tests/MetricsTests/TestMetrics.swift

@@ -4,43 +4,34 @@ import Foundation
 
 internal class TestMetrics: MetricsHandler {
     private let lock = NSLock() // TODO: consider lock per cache?
-    var counters = [String: Counter]()
+    var counters = [Counter]()
-    var recorders = [String: Recorder]()
+    var recorders = [Recorder]()
-    var timers = [String: Timer]()
+    var timers = [Timer]()
 
     public func makeCounter(label: String, dimensions: [(String, String)]) -> Counter {
-        return self.make(label: label, dimensions: dimensions, cache: &self.counters, maker: TestCounter.init)
+        return self.make(label: label, dimensions: dimensions, registry: &self.counters, maker: TestCounter.init)
     }
 
     public func makeRecorder(label: String, dimensions: [(String, String)], aggregate: Bool) -> Recorder {
         let maker = { (label: String, dimensions: [(String, String)]) -> Recorder in
             TestRecorder(label: label, dimensions: dimensions, aggregate: aggregate)
         }
-        return self.make(label: label, dimensions: dimensions, cache: &self.recorders, maker: maker)
+        return self.make(label: label, dimensions: dimensions, registry: &self.recorders, maker: maker)
     }
 
     public func makeTimer(label: String, dimensions: [(String, String)]) -> Timer {
-        return self.make(label: label, dimensions: dimensions, cache: &self.timers, maker: TestTimer.init)
+        return self.make(label: label, dimensions: dimensions, registry: &self.timers, maker: TestTimer.init)
     }
 
-    private func make<Item>(label: String, dimensions: [(String, String)], cache: inout [String: Item], maker: (String, [(String, String)]) -> Item) -> Item {
+    private func make<Item>(label: String, dimensions: [(String, String)], registry: inout [Item], maker: (String, [(String, String)]) -> Item) -> Item {
-        let fqn = self.fqn(label: label, dimensions: dimensions)
-        return self.lock.withLock {
-            if let item = cache[fqn] {
-                return item
-            } else {
         let item = maker(label, dimensions)
-                cache[fqn] = item
+        return self.lock.withLock {
+            registry.append(item)
             return item
         }
     }
 }
 
-    private func fqn(label: String, dimensions: [(String, String)]) -> String {
-        return [[label], dimensions.compactMap { $0.1 }].flatMap { $0 }.joined(separator: ".")
-    }
-}
-
 internal class TestCounter: Counter, Equatable {
     let id: String
     let label: String