fix(patch): Cleanup ARC stats (#134)

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())
+        }
+    }
 }

Sources/Benchmark/ARCStats/ARCStatsProducer.swift

@@ -29,16 +29,18 @@ 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() {
         swift_runtime_set_release_hook(nil, nil)
         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))

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()
         }
     }

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)

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]