From bec70e9b657a9953d976ae5706530b043f11e348 Mon Sep 17 00:00:00 2001 From: Joakim Hassila Date: Tue, 28 Mar 2023 10:26:23 +0200 Subject: [PATCH] fix(patch): Cleanup ARC stats (#134) --- Benchmarks/Basic/BenchmarkRunner+Basic.swift | 15 +++++++++++++++ .../Benchmark/ARCStats/ARCStatsProducer.swift | 8 +++++--- Sources/Benchmark/Benchmark.swift | 18 +++++++++++------- Sources/Benchmark/BenchmarkExecutor.swift | 4 ++++ .../MallocStatsProducer+jemalloc.swift | 7 ++++--- 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/Benchmarks/Basic/BenchmarkRunner+Basic.swift b/Benchmarks/Basic/BenchmarkRunner+Basic.swift index df5883fd..a435f7a0 100644 --- a/Benchmarks/Basic/BenchmarkRunner+Basic.swift +++ b/Benchmarks/Basic/BenchmarkRunner+Basic.swift @@ -59,4 +59,19 @@ let benchmarks = { Benchmark("All metrics", configuration: .init(metrics: BenchmarkMetric.all, skip: true)) { _ in } + + let stats = Statistics(numberOfSignificantDigits: .four) + let measurementCount = 8_340 + + for measurement in (0 ..< measurementCount).reversed() { + stats.add(measurement) + } + + Benchmark("Statistics", + configuration: .init(metrics: BenchmarkMetric.arc + [.wallClock], + scalingFactor: .kilo, maxDuration: .seconds(1))) { benchmark in + for _ in benchmark.scaledIterations { + blackHole(stats.percentiles()) + } + } } diff --git a/Sources/Benchmark/ARCStats/ARCStatsProducer.swift b/Sources/Benchmark/ARCStats/ARCStatsProducer.swift index a183d885..8a8dbd41 100644 --- a/Sources/Benchmark/ARCStats/ARCStatsProducer.swift +++ b/Sources/Benchmark/ARCStats/ARCStatsProducer.swift @@ -29,9 +29,6 @@ final class ARCStatsProducer { swift_runtime_set_retain_hook(retainHook, nil) swift_runtime_set_release_hook(releaseHook, nil) - - ARCStatsProducer.retainCount.store(0, ordering: .relaxed) - ARCStatsProducer.releaseCount.store(0, ordering: .relaxed) } func unhook() { @@ -39,6 +36,11 @@ final class ARCStatsProducer { swift_runtime_set_retain_hook(nil, nil) } + func reset() { + ARCStatsProducer.retainCount.store(0, ordering: .relaxed) + ARCStatsProducer.releaseCount.store(0, ordering: .relaxed) + } + func makeARCStats() -> ARCStats { ARCStats(retainCount: ARCStatsProducer.retainCount.load(ordering: .relaxed), releaseCount: ARCStatsProducer.releaseCount.load(ordering: .relaxed)) diff --git a/Sources/Benchmark/Benchmark.swift b/Sources/Benchmark/Benchmark.swift index aaa2b2b8..de472101 100644 --- a/Sources/Benchmark/Benchmark.swift +++ b/Sources/Benchmark/Benchmark.swift @@ -10,6 +10,8 @@ import Dispatch +// swiftlint: disable file_length + /// Defines a benchmark public final class Benchmark: Codable, Hashable { #if swift(>=5.8) @@ -259,7 +261,7 @@ public final class Benchmark: Codable, Hashable { } /// If the benchmark contains a postample that should not be part of the measurement - /// `startMeasurement` can be called explicitly to define when measurement should begin. + /// `stopMeasurement` can be called explicitly to define when measurement should stop. /// Otherwise the whole benchmark will be measured. public func stopMeasurement() { guard measurementCompleted == false else { // This is to skip the implicit stop if we did an explicit before @@ -284,13 +286,17 @@ public final class Benchmark: Codable, Hashable { // https://forums.swift.org/t/actually-waiting-for-a-task/56230 // Async closures can possibly show false memory leaks possibly due to Swift runtime allocations internal func runAsync() { + guard let asyncClosure else { + fatalError("Tried to runAsync on benchmark instance without any async closure set") + } + let semaphore = DispatchSemaphore(value: 0) // Must do this in a separate thread, otherwise we block the concurrent thread pool DispatchQueue.global(qos: .userInitiated).async { Task { self.startMeasurement() - await self.asyncClosure?(self) + await asyncClosure(self) self.stopMeasurement() semaphore.signal() @@ -304,13 +310,11 @@ public final class Benchmark: Codable, Hashable { @_documentation(visibility: internal) #endif public func run() { - if closure != nil { + if let closure { startMeasurement() - closure?(self) + closure(self) stopMeasurement() - } - - if asyncClosure != nil { + } else { runAsync() } } diff --git a/Sources/Benchmark/BenchmarkExecutor.swift b/Sources/Benchmark/BenchmarkExecutor.swift index 2637d823..01fa2578 100644 --- a/Sources/Benchmark/BenchmarkExecutor.swift +++ b/Sources/Benchmark/BenchmarkExecutor.swift @@ -78,6 +78,8 @@ internal final class BenchmarkExecutor { let initialStartTime = BenchmarkClock.now // Hook that is called before the actual benchmark closure run, so we can capture metrics here + // NB this code may be called twice if the user calls startMeasurement() manually and should + // then reset to a new starting state. benchmark.measurementPreSynchronization = { if mallocStatsRequested { startMallocStats = self.mallocStatsProducer.makeMallocStats() @@ -95,6 +97,7 @@ internal final class BenchmarkExecutor { } // And corresponding hook for then the benchmark has finished and capture finishing metrics here + // This closure will only be called once for a given run though. benchmark.measurementPostSynchronization = { stopTime = BenchmarkClock.now // must be first in closure @@ -276,6 +279,7 @@ internal final class BenchmarkExecutor { let maxPercentage = max(iterationsPercentage, timePercentage) + // Small optimization to not update every single percentage point if Int(maxPercentage) > nextPercentageToUpdateProgressBar { progressBar.setValue(Int(maxPercentage)) nextPercentageToUpdateProgressBar = Int(maxPercentage) + Int.random(in: 3 ... 9) diff --git a/Sources/Benchmark/MallocStats/MallocStatsProducer+jemalloc.swift b/Sources/Benchmark/MallocStats/MallocStatsProducer+jemalloc.swift index 78d983d5..ada415f9 100644 --- a/Sources/Benchmark/MallocStats/MallocStatsProducer+jemalloc.swift +++ b/Sources/Benchmark/MallocStats/MallocStatsProducer+jemalloc.swift @@ -17,9 +17,10 @@ import ExtrasJSON // was used during development to figure out most relevant stats, // Keeping them around as we may want to expand malloc statistics // to become more detailed. - #if swift(>=5.8) - @_documentation(visibility: internal) - #endif + // Disable this for now as it gives unexpected error with Swift 5.7.1 toolchain +// #if swift(>=5.8) +// @_documentation(visibility: internal) +// #endif final class MallocStatsProducer { var threadCacheMIB: [size_t] var epochMIB: [size_t]