remove CachingMetricsHandler

motivation: CachingMetricsHandler represents an optimization that is better handled by implementations than dictated by the API

see: https://forums.swift.org/t/discussion-server-metrics-api/19600/2?u=tomerd

changes:
* remove CachingMetricsHandler
* adjust tests
This commit is contained in:
tomer doron 2019-02-24 18:03:07 -08:00 committed by GitHub
parent 02eeee08d9
commit 2953390316
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 14 additions and 131 deletions

View File

@ -122,8 +122,7 @@ public enum Metrics {
public static func bootstrap(_ handler: MetricsHandler) {
self.lock.withWriterLockVoid {
// using a wrapper to avoid redundant and potentially expensive factory calls
self._handler = CachingMetricsHandler.wrap(handler)
self._handler = handler
}
}
@ -137,75 +136,18 @@ public extension Metrics {
func makeCounter(label: String, dimensions: [(String, String)]) -> Counter {
return Metrics.global.makeCounter(label: label, dimensions: dimensions)
}
@inlinable
func makeRecorder(label: String, dimensions: [(String, String)], aggregate: Bool) -> Recorder {
return Metrics.global.makeRecorder(label: label, dimensions: dimensions, aggregate: aggregate)
}
@inlinable
func makeTimer(label: String, dimensions: [(String, String)]) -> Timer {
return Metrics.global.makeTimer(label: label, dimensions: dimensions)
}
}
private final class CachingMetricsHandler: MetricsHandler {
private let wrapped: MetricsHandler
private var counters = Cache<Counter>()
private var recorders = Cache<Recorder>()
private var timers = Cache<Timer>()
public static func wrap(_ handler: MetricsHandler) -> CachingMetricsHandler {
if let caching = handler as? CachingMetricsHandler {
return caching
} else {
return CachingMetricsHandler(handler)
}
}
private init(_ wrapped: MetricsHandler) {
self.wrapped = wrapped
}
public func makeCounter(label: String, dimensions: [(String, String)]) -> Counter {
return self.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.recorders.getOrSet(label: label, dimensions: dimensions, maker: maker)
}
public func makeTimer(label: String, dimensions: [(String, String)]) -> Timer {
return self.timers.getOrSet(label: label, dimensions: dimensions, maker: self.wrapped.makeTimer)
}
private class Cache<T> {
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 = items[key] {
return item
} else {
let item = maker(label, dimensions)
items[key] = item
return item
}
}
}
private func fqn(label: String, dimensions: [(String, String)]) -> String {
return [[label], dimensions.compactMap { "\($0.0).\($0.1)" }].flatMap { $0 }.joined(separator: ".")
}
}
}
public final class MultiplexMetricsHandler: MetricsHandler {
private let handlers: [MetricsHandler]
public init(handlers: [MetricsHandler]) {

View File

@ -43,7 +43,7 @@ class MetricsTests: XCTestCase {
let name = "counter-\(NSUUID().uuidString)"
let value = Int.random(in: Int.min ... Int.max)
Metrics.global.withCounter(label: name) { $0.increment(value) }
let counter = Metrics.global.makeCounter(label: name) as! TestCounter
let counter = metrics.counters[name] as! TestCounter
XCTAssertEqual(counter.values.count, 1, "expected number of entries to match")
XCTAssertEqual(counter.values[0].1, Int64(value), "expected value to match")
}
@ -105,7 +105,7 @@ class MetricsTests: XCTestCase {
let name = "recorder-\(NSUUID().uuidString)"
let value = Double.random(in: Double(Int.min) ... Double(Int.max))
Metrics.global.withRecorder(label: name) { $0.record(value) }
let recorder = Metrics.global.makeRecorder(label: name) as! TestRecorder
let recorder = metrics.recorders[name] as! TestRecorder
XCTAssertEqual(recorder.values.count, 1, "expected number of entries to match")
XCTAssertEqual(recorder.values[0].1, value, "expected value to match")
}
@ -137,7 +137,7 @@ class MetricsTests: XCTestCase {
let name = "timer-\(NSUUID().uuidString)"
let value = Int64.random(in: Int64.min ... Int64.max)
Metrics.global.withTimer(label: name) { $0.recordNanoseconds(value) }
let timer = Metrics.global.makeTimer(label: name) as! TestTimer
let timer = metrics.timers[name] as! TestTimer
XCTAssertEqual(timer.values.count, 1, "expected number of entries to match")
XCTAssertEqual(timer.values[0].1, value, "expected value to match")
}
@ -192,7 +192,7 @@ class MetricsTests: XCTestCase {
let name = "gauge-\(NSUUID().uuidString)"
let value = Double.random(in: -1000 ... 1000)
Metrics.global.withGauge(label: name) { $0.record(value) }
let recorder = Metrics.global.makeGauge(label: name) as! TestRecorder
let recorder = metrics.recorders[name] as! TestRecorder
XCTAssertEqual(recorder.values.count, 1, "expected number of entries to match")
XCTAssertEqual(recorder.values[0].1, value, "expected value to match")
}
@ -208,69 +208,10 @@ class MetricsTests: XCTestCase {
counter.increment(value)
}
handlers.forEach { handler in
let counter = handler.counters[0] as! TestCounter
let counter = handler.counters.first?.1 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 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)
// run the test
let name = "counter-\(NSUUID().uuidString)"
let dimensions = [("foo", "bar"), ("baz", "quk")]
let counter = Metrics.global.makeCounter(label: name, dimensions: dimensions) as! TestCounter
XCTAssertEqual(counter.label, name, "expected dimensions to match")
XCTAssertEqual(counter.dimensions.description, dimensions.description, "expected dimensions to match")
// same
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")
}
}

View File

@ -26,7 +26,7 @@ class MetricsExtensionsTests: XCTestCase {
Metrics.global.timed(label: name) {
Thread.sleep(forTimeInterval: delay)
}
let timer = Metrics.global.makeTimer(label: name) as! TestTimer
let timer = metrics.timers[name] as! TestTimer
XCTAssertEqual(1, timer.values.count, "expected number of entries to match")
XCTAssertGreaterThan(timer.values[0].1, Int64(delay * 1_000_000_000), "expected delay to match")
}

View File

@ -18,9 +18,9 @@ import Foundation
internal class TestMetrics: MetricsHandler {
private let lock = NSLock() // TODO: consider lock per cache?
var counters = [Counter]()
var recorders = [Recorder]()
var timers = [Timer]()
var counters = [String: Counter]()
var recorders = [String: Recorder]()
var timers = [String: Timer]()
public func makeCounter(label: String, dimensions: [(String, String)]) -> Counter {
return self.make(label: label, dimensions: dimensions, registry: &self.counters, maker: TestCounter.init)
@ -37,10 +37,10 @@ internal class TestMetrics: MetricsHandler {
return self.make(label: label, dimensions: dimensions, registry: &self.timers, maker: TestTimer.init)
}
private func make<Item>(label: String, dimensions: [(String, String)], registry: inout [Item], maker: (String, [(String, String)]) -> Item) -> Item {
private func make<Item>(label: String, dimensions: [(String, String)], registry: inout [String: Item], maker: (String, [(String, String)]) -> Item) -> Item {
let item = maker(label, dimensions)
return self.lock.withLock {
registry.append(item)
registry[label] = item
return item
}
}