From 4f950d680a0df483ddd1e0fde1484a223fad7790 Mon Sep 17 00:00:00 2001 From: MahdiBM Date: Mon, 14 Jul 2025 23:37:28 +0330 Subject: [PATCH 1/5] Run swift-format --- .../Basic/BenchmarkRunner+Basic.swift | 2 +- Package.swift | 4 +- .../BenchmarkBoilerplateGenerator.swift | 2 +- .../BenchmarkCommandPlugin.swift | 10 ++--- .../BenchmarkTool+Baselines.swift | 43 +++++++++---------- .../BenchmarkTool+CreateBenchmark.swift | 16 +++---- ...chmarkTool+Export+InfluxCSVFormatter.swift | 3 +- .../BenchmarkTool+Export+JMHFormatter.swift | 4 +- .../BenchmarkTool/BenchmarkTool+Machine.swift | 2 +- .../BenchmarkTool+Operations.swift | 6 +-- ...chmarkTool+ReadP90AbsoluteThresholds.swift | 2 +- Plugins/BenchmarkTool/BenchmarkTool.swift | 12 +++--- .../BenchmarkTool/FilePath+Additions.swift | 2 +- .../Benchmark+ConvenienceInitializers.swift | 4 +- Sources/Benchmark/Benchmark.swift | 20 ++++----- Sources/Benchmark/BenchmarkExecutor.swift | 18 ++++---- Sources/Benchmark/BenchmarkInternals.swift | 8 ++-- Sources/Benchmark/BenchmarkMetric.swift | 8 ++-- Sources/Benchmark/BenchmarkResult.swift | 29 +++++++------ Sources/Benchmark/BenchmarkRunner.swift | 2 +- Sources/Benchmark/Blackhole.swift | 4 +- .../Benchmark/MallocStats/MallocStats.swift | 2 +- .../OperatingSystemStatsProducer+Linux.swift | 2 +- .../Benchmark/Progress/ProgressElements.swift | 2 +- Sources/Benchmark/Statistics.swift | 16 +++---- .../BenchmarkTests/BenchmarkRunnerTests.swift | 2 +- 26 files changed, 114 insertions(+), 111 deletions(-) diff --git a/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift b/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift index 3e7b4114..837c0cb1 100644 --- a/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift +++ b/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift @@ -125,7 +125,7 @@ let benchmarks: @Sendable () -> Void = { } } - let parameterization = (0...5).map { 1 << $0 } // 1, 2, 4, ... + let parameterization = (0...5).map { 1 << $0 } // 1, 2, 4, ... parameterization.forEach { count in Benchmark("Parameterized", configuration: .init(tags: ["count": count.description])) { benchmark in diff --git a/Package.swift b/Package.swift index e6790598..72969970 100644 --- a/Package.swift +++ b/Package.swift @@ -117,7 +117,7 @@ let package = Package( ) // Check if this is a SPI build, then we need to disable jemalloc for macOS -let macOSSPIBuild: Bool // Disables jemalloc for macOS SPI builds as the infrastructure doesn't have jemalloc there +let macOSSPIBuild: Bool // Disables jemalloc for macOS SPI builds as the infrastructure doesn't have jemalloc there #if canImport(Darwin) if let spiBuildEnvironment = ProcessInfo.processInfo.environment["SPI_BUILD"], spiBuildEnvironment == "1" { @@ -144,7 +144,7 @@ var dependencies: [PackageDescription.Target.Dependency] = [ "BenchmarkShared", ] -if macOSSPIBuild == false { // jemalloc always disable for macOSSPIBuild +if macOSSPIBuild == false { // jemalloc always disable for macOSSPIBuild if let disableJemalloc, disableJemalloc != "false", disableJemalloc != "0" { print("Jemalloc disabled through environment variable.") } else { diff --git a/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift b/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift index 2da0884d..03bcffd8 100644 --- a/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift +++ b/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift @@ -20,7 +20,7 @@ struct Benchmark: AsyncParsableCommand { var output: String mutating func run() async throws { - let outputPath = FilePath(output) // package + let outputPath = FilePath(output) // package var boilerplate = """ import Benchmark diff --git a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift index 319c0586..ac3657d6 100644 --- a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift +++ b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift @@ -37,7 +37,7 @@ import Glibc let specifiedTargets = try argumentExtractor.extractSpecifiedTargets(in: context.package, withOption: "target") let skipTargets = try argumentExtractor.extractSpecifiedTargets(in: context.package, withOption: "skip-target") let outputFormats = argumentExtractor.extractOption(named: "format") - let pathSpecified = argumentExtractor.extractOption(named: "path") // export path + let pathSpecified = argumentExtractor.extractOption(named: "path") // export path let quietRunning = argumentExtractor.extractFlag(named: "quiet") let noProgress = argumentExtractor.extractFlag(named: "no-progress") let checkAbsoluteThresholdsPath = argumentExtractor.extractOption(named: "check-absolute-path") @@ -139,16 +139,16 @@ import Glibc ) throw MyError.invalidArgument } - } catch { // We will throw if we can use the target name (it's unused!) + } catch { // We will throw if we can use the target name (it's unused!) } } let swiftSourceModuleTargets: [SwiftSourceModuleTarget] - var shouldBuildTargets = true // We don't rebuild the targets when we dont need to execute them, e.g. baseline read/compare + var shouldBuildTargets = true // We don't rebuild the targets when we dont need to execute them, e.g. baseline read/compare let packageBenchmarkIdentifier = "package-benchmark" let benchmarkToolName = "BenchmarkTool" - let benchmarkTool: PackagePlugin.Path // = try context.tool(named: benchmarkToolName) + let benchmarkTool: PackagePlugin.Path // = try context.tool(named: benchmarkToolName) var args: [String] = [ benchmarkToolName, @@ -435,7 +435,7 @@ import Glibc } let buildResult = try packageManager.build( - .product(target.name), // .all(includingTests: false), + .product(target.name), // .all(includingTests: false), parameters: .init(configuration: .release) ) diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift b/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift index 9e97e2b1..40cdcaaf 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift @@ -33,8 +33,8 @@ struct BenchmarkMachine: Codable, Equatable { var hostname: String var processors: Int - var processorType: String // e.g. arm64e - var memory: Int // in GB + var processorType: String // e.g. arm64e + var memory: Int // in GB var kernelVersion: String public static func == (lhs: BenchmarkMachine, rhs: BenchmarkMachine) -> Bool { @@ -48,8 +48,8 @@ struct BenchmarkIdentifier: Codable, Hashable { self.name = name } - var target: String // The name of the executable benchmark target id - var name: String // The name of the benchmark + var target: String // The name of the executable benchmark target id + var name: String // The name of the benchmark public func hash(into hasher: inout Hasher) { hasher.combine(target) @@ -178,7 +178,7 @@ let baselinesDirectory: String = ".benchmarkBaselines" extension BenchmarkTool { func printAllBaselines() { var storagePath = FilePath(baselineStoragePath) - storagePath.append(baselinesDirectory) // package/.benchmarkBaselines + storagePath.append(baselinesDirectory) // package/.benchmarkBaselines for file in storagePath.directoryEntries { if file.ends(with: ".") == false, file.ends(with: "..") == false @@ -206,7 +206,7 @@ extension BenchmarkTool { var storagePath = FilePath(baselineStoragePath) let filemanager = FileManager.default - storagePath.append(baselinesDirectory) // package/.benchmarkBaselines + storagePath.append(baselinesDirectory) // package/.benchmarkBaselines for file in storagePath.directoryEntries { if file.ends(with: ".") == false, file.ends(with: "..") == false @@ -284,14 +284,14 @@ extension BenchmarkTool { │ └── ... └── ... */ - var outputPath = FilePath(baselineStoragePath) // package - var subPath = FilePath() // subpath rooted in package used for directory creation + var outputPath = FilePath(baselineStoragePath) // package + var subPath = FilePath() // subpath rooted in package used for directory creation - subPath.append(baselinesDirectory) // package/.benchmarkBaselines - subPath.append("\(target)") // package/.benchmarkBaselines/myTarget1 - subPath.append(baselineName) // package/.benchmarkBaselines/myTarget1/named1 + subPath.append(baselinesDirectory) // package/.benchmarkBaselines + subPath.append("\(target)") // package/.benchmarkBaselines/myTarget1 + subPath.append(baselineName) // package/.benchmarkBaselines/myTarget1/named1 - outputPath.createSubPath(subPath) // Create destination subpath if needed + outputPath.createSubPath(subPath) // Create destination subpath if needed outputPath.append(subPath.components) @@ -348,13 +348,13 @@ extension BenchmarkTool { baselineIdentifier: String? = nil ) throws -> BenchmarkBaseline? { var path = FilePath(baselineStoragePath) - path.append(baselinesDirectory) // package/.benchmarkBaselines - path.append(FilePath.Component(target)!) // package/.benchmarkBaselines/myTarget1 + path.append(baselinesDirectory) // package/.benchmarkBaselines + path.append(FilePath.Component(target)!) // package/.benchmarkBaselines/myTarget1 if let baselineIdentifier { - path.append(baselineIdentifier) // package/.benchmarkBaselines/myTarget1/named1 + path.append(baselineIdentifier) // package/.benchmarkBaselines/myTarget1/named1 } else { - path.append("default") // // package/.benchmarkBaselines/myTarget1/default + path.append("default") // // package/.benchmarkBaselines/myTarget1/default } if let hostIdentifier { @@ -376,7 +376,7 @@ extension BenchmarkTool { let bufferSize = 16 * 1_024 * 1_024 var done = false - while done == false { // readBytes.count < bufferLength { + while done == false { // readBytes.count < bufferLength { let nextBytes = try [UInt8](unsafeUninitializedCapacity: bufferSize) { buf, count in count = try fd.read(into: UnsafeMutableRawBufferPointer(buf)) if count == 0 { @@ -396,7 +396,7 @@ extension BenchmarkTool { print("Failed to close fd for \(path) after reading.") } } catch { - if errno != ENOENT { // file not found is ok, e.g. when no baselines exist + if errno != ENOENT { // file not found is ok, e.g. when no baselines exist print("Failed to open file \(path), errno = [\(errno)]") } } @@ -449,8 +449,7 @@ extension BenchmarkBaseline: Equatable { benchmarks, name: lhsBenchmarkIdentifier.name, target: lhsBenchmarkIdentifier.target, - metric: lhsBenchmarkResult.metric, - defaultThresholds: lhsBenchmarkResult.thresholds ?? BenchmarkThresholds.default + metric: lhsBenchmarkResult.metric ) let deviationResults = lhsBenchmarkResult.deviationsComparedWith( @@ -523,11 +522,11 @@ extension BenchmarkBaseline: Equatable { for (lhsBenchmarkIdentifier, lhsBenchmarkResults) in lhs.results { for lhsBenchmarkResult in lhsBenchmarkResults { - guard let rhsResults = rhs.results.first(where: { $0.key == lhsBenchmarkIdentifier }) else { // We couldn't find a result for one of the tests + guard let rhsResults = rhs.results.first(where: { $0.key == lhsBenchmarkIdentifier }) else { // We couldn't find a result for one of the tests return false } guard let rhsBenchmarkResult = rhsResults.value.first(where: { $0.metric == lhsBenchmarkResult.metric }) - else { // We couldn't find the specific metric + else { // We couldn't find the specific metric return false } if lhsBenchmarkResult != rhsBenchmarkResult { diff --git a/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift b/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift index 86c7c574..e48f8f61 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift @@ -44,9 +44,9 @@ extension BenchmarkTool { ] """ - var outputPath = FilePath(baselineStoragePath) // package - var subPath = FilePath() // subpath rooted in package used for directory creation - subPath.append("Package.swift") // package/Benchmarks/targetName + var outputPath = FilePath(baselineStoragePath) // package + var subPath = FilePath() // subpath rooted in package used for directory creation + subPath.append("Package.swift") // package/Benchmarks/targetName outputPath.append(subPath.components) print("Adding new executable target \(targetName) to \(outputPath.description)") @@ -110,13 +110,13 @@ extension BenchmarkTool { """ - var outputPath = FilePath(baselineStoragePath) // package - var subPath = FilePath() // subpath rooted in package used for directory creation + var outputPath = FilePath(baselineStoragePath) // package + var subPath = FilePath() // subpath rooted in package used for directory creation - subPath.append(benchmarksDirectory) // package/Benchmarks - subPath.append("\(targetName)") // package/Benchmarks/targetName + subPath.append(benchmarksDirectory) // package/Benchmarks + subPath.append("\(targetName)") // package/Benchmarks/targetName - outputPath.createSubPath(subPath) // Create destination subpath if needed + outputPath.createSubPath(subPath) // Create destination subpath if needed outputPath.append(subPath.components) outputPath.append("\(targetName).swift") diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Export+InfluxCSVFormatter.swift b/Plugins/BenchmarkTool/BenchmarkTool+Export+InfluxCSVFormatter.swift index f8b81ca5..e0b8921a 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Export+InfluxCSVFormatter.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Export+InfluxCSVFormatter.swift @@ -54,7 +54,8 @@ class InfluxCSVFormatter { let memory = machine.memory if header { - let dataTypeHeader = "#datatype tag,tag,tag,tag,tag,tag,tag,tag,tag,double,double,double,long,long,dateTime\n" + let dataTypeHeader = + "#datatype tag,tag,tag,tag,tag,tag,tag,tag,tag,double,double,double,long,long,dateTime\n" finalFileFormat.append(dataTypeHeader) let headers = "measurement,hostName,processoryType,processors,memory,kernelVersion,metric,unit,test,percentile,value,test_average,iterations,warmup_iterations,time\n" diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift b/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift index 15d00576..40d00caf 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift @@ -56,7 +56,7 @@ extension JMHPrimaryMetric { if result.metric.countable { scoreUnit = result.metric == .throughput ? "# / s" : "#" } else { - scoreUnit = "μs" // result.timeUnits.description + scoreUnit = "μs" // result.timeUnits.description } rawData = [recordedValues] } @@ -66,7 +66,7 @@ extension BenchmarkTool { func convertToJMH(_ baseline: BenchmarkBaseline) throws -> String { var resultString = "" var jmhElements: [JMHElement] = [] - var secondaryMetrics: [String: JMHPrimaryMetric] = [:] // could move to OrderedDictionary for consistent output + var secondaryMetrics: [String: JMHPrimaryMetric] = [:] // could move to OrderedDictionary for consistent output baseline.targets.forEach { benchmarkTarget in diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift b/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift index 08934f49..341add71 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift @@ -23,7 +23,7 @@ import Glibc extension BenchmarkTool { func benchmarkMachine() -> BenchmarkMachine { let processors = sysconf(Int32(_SC_NPROCESSORS_ONLN)) - let memory = sysconf(Int32(_SC_PHYS_PAGES)) / 1_024 * sysconf(Int32(_SC_PAGESIZE)) / (1_024 * 1_024) // avoid overflow + let memory = sysconf(Int32(_SC_PHYS_PAGES)) / 1_024 * sysconf(Int32(_SC_PAGESIZE)) / (1_024 * 1_024) // avoid overflow var uuname = utsname() _ = uname(&uuname) diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift b/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift index 28275c8d..7f573afc 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift @@ -129,7 +129,7 @@ extension BenchmarkTool { return } - if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare + if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare currentBaseline.results.keys.forEach { baselineKey in if let benchmark: Benchmark = .init(baselineKey.name, closure: { _ in }) { benchmark.target = baselineKey.target @@ -282,7 +282,7 @@ extension BenchmarkTool { return } - if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare + if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare currentBaseline.results.keys.forEach { baselineKey in if let benchmark: Benchmark = .init(baselineKey.name, closure: { _ in }) { benchmark.target = baselineKey.target @@ -302,7 +302,7 @@ extension BenchmarkTool { var p90Thresholds: [BenchmarkIdentifier: [BenchmarkMetric: BenchmarkThresholds.AbsoluteThreshold]] = [:] - if let benchmarkPath = checkAbsolutePath { // load statically defined thresholds for .p90 + if let benchmarkPath = checkAbsolutePath { // load statically defined thresholds for .p90 benchmarks.forEach { benchmark in if let thresholds = BenchmarkTool.makeBenchmarkThresholds( path: benchmarkPath, diff --git a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift index 5fad206b..066dd554 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift @@ -89,7 +89,7 @@ extension BenchmarkTool { print("Failed to close fd for \(path) after reading.") } } catch { - if errno != ENOENT { // file not found is ok, e.g. no thresholds found, then silently return nil + if errno != ENOENT { // file not found is ok, e.g. no thresholds found, then silently return nil print("Failed to open file \(path), errno = [\(errno)] \(Errno(rawValue: errno).description)") } } diff --git a/Plugins/BenchmarkTool/BenchmarkTool.swift b/Plugins/BenchmarkTool/BenchmarkTool.swift index a2ecf655..6bb6233c 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool.swift @@ -28,7 +28,7 @@ enum BenchmarkOperation: String, ExpressibleByArgument { case thresholds case list case run - case query // query all benchmarks from target, used internally in tool + case query // query all benchmarks from target, used internally in tool case `init` } @@ -127,7 +127,7 @@ struct BenchmarkTool: AsyncParsableCommand { var outputFD: CInt = 0 var benchmarks: [Benchmark] = [] - var benchmarkBaselines: [BenchmarkBaseline] = [] // The baselines read from disk, merged + current run if needed + var benchmarkBaselines: [BenchmarkBaseline] = [] // The baselines read from disk, merged + current run if needed var comparisonBaseline: BenchmarkBaseline? var checkBaseline: BenchmarkBaseline? @@ -192,9 +192,9 @@ struct BenchmarkTool: AsyncParsableCommand { mutating func readBaselines() throws { func readBaseline(_ baselineName: String) throws -> BenchmarkBaseline? { // read all specified baselines - var readBaselines: [BenchmarkBaseline] = [] // The baselines read from disk + var readBaselines: [BenchmarkBaseline] = [] // The baselines read from disk - try targets.forEach { target in // read from all the targets (baselines are stored separately) + try targets.forEach { target in // read from all the targets (baselines are stored separately) let currentBaseline = try read(target: target, baselineIdentifier: baselineName) if let currentBaseline { @@ -214,7 +214,7 @@ struct BenchmarkTool: AsyncParsableCommand { return nil } - try baseline.forEach { baselineName in // for all specified baselines at command line + try baseline.forEach { baselineName in // for all specified baselines at command line if let baseline = try readBaseline(baselineName) { benchmarkBaselines.append(baseline) } else { @@ -401,7 +401,7 @@ struct BenchmarkTool: AsyncParsableCommand { case .`init`: fatalError("Should never come here") case .query: - try queryBenchmarks(benchmarkPath) // Get all available benchmarks first + try queryBenchmarks(benchmarkPath) // Get all available benchmarks first case .list: try listBenchmarks() case .baseline, .thresholds, .run: diff --git a/Plugins/BenchmarkTool/FilePath+Additions.swift b/Plugins/BenchmarkTool/FilePath+Additions.swift index 7659e7cc..228bc7a6 100644 --- a/Plugins/BenchmarkTool/FilePath+Additions.swift +++ b/Plugins/BenchmarkTool/FilePath+Additions.swift @@ -38,7 +38,7 @@ public extension FilePath { } catch { print("failed close directory") } } catch { switch errno { - case ENOENT: // doesn't exist, let's create it + case ENOENT: // doesn't exist, let's create it if mkdir(creationPath.string, S_IRWXU | S_IRWXG | S_IRWXO) == -1 { if errno == EPERM { print("Lacking permissions to write to \(creationPath)") diff --git a/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift b/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift index e40eb651..61212f21 100644 --- a/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift +++ b/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift @@ -17,7 +17,7 @@ public extension Benchmark { teardown: BenchmarkTeardownHook? = nil ) { self.init(name, configuration: configuration) { benchmark in - let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast + let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast closure(benchmark, setupResult) } teardown: { try await teardown?() @@ -46,7 +46,7 @@ public extension Benchmark { teardown: BenchmarkTeardownHook? = nil ) { self.init(name, configuration: configuration) { benchmark in - let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast + let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast await closure(benchmark, setupResult) } teardown: { try await teardown?() diff --git a/Sources/Benchmark/Benchmark.swift b/Sources/Benchmark/Benchmark.swift index 44e811f2..c0bc7f44 100644 --- a/Sources/Benchmark/Benchmark.swift +++ b/Sources/Benchmark/Benchmark.swift @@ -14,7 +14,7 @@ import Foundation // swiftlint:disable file_length identifier_name /// Defines a benchmark -public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type_body_length +public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type_body_length @_documentation(visibility: internal) public typealias BenchmarkClosure = (_ benchmark: Benchmark) -> Void @_documentation(visibility: internal) @@ -36,11 +36,11 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type @_documentation(visibility: internal) @ThreadSafeProperty(wrappedValue: nil, lock: setupTeardownLock) - public static var _startupHook: BenchmarkSetupHook? // Should be removed when going to 2.0, just kept for API compatiblity + public static var _startupHook: BenchmarkSetupHook? // Should be removed when going to 2.0, just kept for API compatiblity @_documentation(visibility: internal) @ThreadSafeProperty(wrappedValue: nil, lock: setupTeardownLock) - public static var _shutdownHook: BenchmarkTeardownHook? // Should be removed when going to 2.0, just kept for API compatiblity + public static var _shutdownHook: BenchmarkTeardownHook? // Should be removed when going to 2.0, just kept for API compatiblity @_documentation(visibility: internal) @ThreadSafeProperty(wrappedValue: nil, lock: setupTeardownLock) @@ -111,7 +111,7 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type public static var checkAbsoluteThresholds = false @_documentation(visibility: internal) - public static var benchmarks: [Benchmark] = [] // Bookkeeping of all registered benchmarks + public static var benchmarks: [Benchmark] = [] // Bookkeeping of all registered benchmarks /// The name of the benchmark without any of the tags appended public var baseName: String @@ -150,9 +150,9 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type @_documentation(visibility: internal) public var executablePath: String? /// closure: The actual benchmark closure that will be measured - var closure: BenchmarkClosure? // The actual benchmark to run + var closure: BenchmarkClosure? // The actual benchmark to run /// asyncClosure: The actual benchmark (async) closure that will be measured - var asyncClosure: BenchmarkAsyncClosure? // The actual benchmark to run + var asyncClosure: BenchmarkAsyncClosure? // The actual benchmark to run // setup/teardown hooks for the instance var setup: BenchmarkSetupHook? var teardown: BenchmarkTeardownHook? @@ -205,8 +205,8 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type } #endif - static var testSkipBenchmarkRegistrations = false // true in test to avoid bench registration fail - var measurementCompleted = false // Keep track so we skip multiple 'end of measurement' + static var testSkipBenchmarkRegistrations = false // true in test to avoid bench registration fail + var measurementCompleted = false // Keep track so we skip multiple 'end of measurement' enum CodingKeys: String, CodingKey { case baseName = "name" @@ -406,7 +406,7 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type } private func _stopMeasurement(_ explicitStartStop: Bool) { - guard measurementCompleted == false else { // This is to skip the implicit stop if we did an explicit before + guard measurementCompleted == false else { // This is to skip the implicit stop if we did an explicit before return } @@ -560,7 +560,7 @@ public extension Benchmark { /// } /// } /// ``` - @_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf + @_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf static func blackHole(_: some Any) {} } diff --git a/Sources/Benchmark/BenchmarkExecutor.swift b/Sources/Benchmark/BenchmarkExecutor.swift index 99e07086..6c640ff6 100644 --- a/Sources/Benchmark/BenchmarkExecutor.swift +++ b/Sources/Benchmark/BenchmarkExecutor.swift @@ -14,7 +14,7 @@ import OSLog // swiftlint:disable file_length -struct BenchmarkExecutor { // swiftlint:disable:this type_body_length +struct BenchmarkExecutor { // swiftlint:disable:this type_body_length init(quiet: Bool = false) { self.quiet = quiet } @@ -107,7 +107,7 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length let initialStartTime = BenchmarkClock.now // 'Warmup' to remove initial mallocs from stats in p100 - _ = MallocStatsProducer.makeMallocStats() // baselineMallocStats + _ = MallocStatsProducer.makeMallocStats() // baselineMallocStats // Calculate typical sys call check overhead and deduct that to get 'clean' stats for the actual benchmark var operatingSystemStatsOverhead = OperatingSystemStats() @@ -129,7 +129,7 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length for _ in 0.. .zero { // macOS sometimes gives us identical timestamps so let's skip those. + if runningTime > .zero { // macOS sometimes gives us identical timestamps so let's skip those. let nanoSeconds = runningTime.nanoseconds() statistics[BenchmarkMetric.wallClock.index].add(Int(nanoSeconds)) @@ -228,10 +228,10 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length let objectAllocDelta = stopARCStats.objectAllocCount - startARCStats.objectAllocCount statistics[BenchmarkMetric.objectAllocCount.index].add(Int(objectAllocDelta)) - let retainDelta = stopARCStats.retainCount - startARCStats.retainCount - 1 // due to some ARC traffic in the path + let retainDelta = stopARCStats.retainCount - startARCStats.retainCount - 1 // due to some ARC traffic in the path statistics[BenchmarkMetric.retainCount.index].add(Int(retainDelta)) - let releaseDelta = stopARCStats.releaseCount - startARCStats.releaseCount - 1 // due to some ARC traffic in the path + let releaseDelta = stopARCStats.releaseCount - startARCStats.releaseCount - 1 // due to some ARC traffic in the path statistics[BenchmarkMetric.releaseCount.index].add(Int(releaseDelta)) statistics[BenchmarkMetric.retainReleaseDelta.index] @@ -399,7 +399,7 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length iterations += 1 - if iterations < 1_000 || iterations.isMultiple(of: 500) { // only update for low iteration count benchmarks, else 1/500 + if iterations < 1_000 || iterations.isMultiple(of: 500) { // only update for low iteration count benchmarks, else 1/500 if var progressBar { let iterationsPercentage = 100.0 * Double(iterations) / Double(benchmark.configuration.maxIterations) diff --git a/Sources/Benchmark/BenchmarkInternals.swift b/Sources/Benchmark/BenchmarkInternals.swift index 1c624116..e942c577 100644 --- a/Sources/Benchmark/BenchmarkInternals.swift +++ b/Sources/Benchmark/BenchmarkInternals.swift @@ -17,7 +17,7 @@ public enum BenchmarkCommandRequest: Codable { case list case run(benchmark: Benchmark) - case end // exit the benchmark + case end // exit the benchmark } // Replies from benchmark under measure to benchmark runner @@ -25,10 +25,10 @@ public enum BenchmarkCommandRequest: Codable { public enum BenchmarkCommandReply: Codable { case list(benchmark: Benchmark) case ready - case result(benchmark: Benchmark, results: [BenchmarkResult]) // receives results from built-in metric collectors + case result(benchmark: Benchmark, results: [BenchmarkResult]) // receives results from built-in metric collectors case run - case end // end of query for list/result - case error(_ description: String) // error while performing operation (e.g. 'run') + case end // end of query for list/result + case error(_ description: String) // error while performing operation (e.g. 'run') } // swiftlint:enable all diff --git a/Sources/Benchmark/BenchmarkMetric.swift b/Sources/Benchmark/BenchmarkMetric.swift index b5d06096..475f49f3 100644 --- a/Sources/Benchmark/BenchmarkMetric.swift +++ b/Sources/Benchmark/BenchmarkMetric.swift @@ -96,7 +96,7 @@ public extension BenchmarkMetric { public extension BenchmarkMetric { /// A constant that states whether larger or smaller measurements, relative to a set baseline, indicate better performance. - enum Polarity: Codable, Sendable { // same naming as XCTest uses, polarity is known for all metrics except custom + enum Polarity: Codable, Sendable { // same naming as XCTest uses, polarity is known for all metrics except custom /// A performance measurement where a larger value, relative to a set baseline, indicates better performance. case prefersLarger /// A performance measurement where a smaller value, relative to a set baseline, indicates better performance. @@ -279,7 +279,7 @@ public extension BenchmarkMetric { case .instructions: return 28 default: - return 0 // custom payloads must be stored in dictionary + return 0 // custom payloads must be stored in dictionary } } @@ -288,7 +288,7 @@ public extension BenchmarkMetric { // Used by the Benchmark Executor for efficient indexing into results @_documentation(visibility: internal) - func metricFor(index: Int) -> BenchmarkMetric { // swiftlint:disable:this cyclomatic_complexity function_body_length + func metricFor(index: Int) -> BenchmarkMetric { // swiftlint:disable:this cyclomatic_complexity function_body_length switch index { case 1: return .cpuUser @@ -355,7 +355,7 @@ public extension BenchmarkMetric { @_documentation(visibility: internal) public extension BenchmarkMetric { - var rawDescription: String { // As we can't have raw values due to custom support, we do this... + var rawDescription: String { // As we can't have raw values due to custom support, we do this... switch self { case .cpuUser: return "cpuUser" diff --git a/Sources/Benchmark/BenchmarkResult.swift b/Sources/Benchmark/BenchmarkResult.swift index 80350f2c..47f19770 100644 --- a/Sources/Benchmark/BenchmarkResult.swift +++ b/Sources/Benchmark/BenchmarkResult.swift @@ -31,7 +31,7 @@ public enum BenchmarkTimeUnits: String, Codable, CustomStringConvertible, CaseIt case seconds case kiloseconds case megaseconds - case automatic // will pick time unit above automatically + case automatic // will pick time unit above automatically public var factor: Int { switch self { case .nanoseconds: @@ -43,7 +43,7 @@ public enum BenchmarkTimeUnits: String, Codable, CustomStringConvertible, CaseIt case .seconds: return 1 case .kiloseconds: - return 2 // Yeah, not right but we need to refactor to get rid of this, works for now + return 2 // Yeah, not right but we need to refactor to get rid of this, works for now case .megaseconds: return 3 case .automatic: @@ -98,7 +98,7 @@ public enum BenchmarkUnits: Int, Codable, CustomStringConvertible, CaseIterable case giga = 1_000_000_000 case tera = 1_000_000_000_000 case peta = 1_000_000_000_000_000 - case automatic // will pick unit above automatically + case automatic // will pick unit above automatically public var description: String { switch self { @@ -169,17 +169,17 @@ public extension BenchmarkTimeUnits { /// Use a scaling factor when running your short benchmarks to provide greater numerical stability to the results. public enum BenchmarkScalingFactor: Int, Codable { /// No scaling factor, the raw count of iterations. - case one = 1 // e.g. nanoseconds, or count + case one = 1 // e.g. nanoseconds, or count /// Scaling factor of 1e03. - case kilo = 1_000 // microseconds + case kilo = 1_000 // microseconds /// Scaling factor of 1e06. - case mega = 1_000_000 // milliseconds + case mega = 1_000_000 // milliseconds /// Scaling factor of 1e09. - case giga = 1_000_000_000 // seconds + case giga = 1_000_000_000 // seconds /// Scaling factor of 1e12. - case tera = 1_000_000_000_000 // 1K seconds + case tera = 1_000_000_000_000 // 1K seconds /// Scaling factor of 1e15. - case peta = 1_000_000_000_000_000 // 1M + case peta = 1_000_000_000_000_000 // 1M public var description: String { switch self { @@ -279,7 +279,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { return .microseconds case .giga: return .nanoseconds - case .tera, .peta: // shouldn't be possible as tera is only used internally to present scaled up throughput + case .tera, .peta: // shouldn't be possible as tera is only used internally to present scaled up throughput break } default: @@ -312,7 +312,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { return y * x } else if n.isMultiple(of: 2) { return expBySq(y, x * x, n / 2) - } else { // n is odd + } else { // n is odd return expBySq(y * x, x * x, (n - 1) / 2) } } @@ -493,6 +493,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { _ rhs: Int, _ percentile: Self.Percentile, _ thresholds: BenchmarkThresholds, + _ scalingFactor: Statistics.Units, _ thresholdResults: inout ThresholdDeviations, _ name: String = "unknown name", _ target: String = "unknown target" @@ -513,7 +514,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { difference: Int(Statistics.roundToDecimalplaces(relativeDifference, 1)), differenceThreshold: Int(threshold), relative: true, - units: Statistics.Units(timeUnits) + units: scalingFactor ) if relativeDifference > threshold { thresholdResults.regressions.append(deviation) @@ -533,7 +534,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { difference: normalize(absoluteDifference), differenceThreshold: normalize(threshold), relative: false, - units: Statistics.Units(timeUnits) + units: scalingFactor ) if absoluteDifference > threshold { @@ -567,6 +568,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { rhsPercentiles[percentile], Self.Percentile(rawValue: percentile)!, thresholds, + lhs.statistics.units(), &thresholdResults, name, target @@ -592,6 +594,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { p90Threshold, .p90, thresholds, + statistics.units(), &thresholdResults, name, target diff --git a/Sources/Benchmark/BenchmarkRunner.swift b/Sources/Benchmark/BenchmarkRunner.swift index 8ed30363..4946da62 100644 --- a/Sources/Benchmark/BenchmarkRunner.swift +++ b/Sources/Benchmark/BenchmarkRunner.swift @@ -119,7 +119,7 @@ public struct BenchmarkRunner: AsyncParsableCommand, BenchmarkRunnerReadWrite { let suppressor = OutputSuppressor() while true { - if debug { // in debug mode we run all benchmarks matching filter/skip specified + if debug { // in debug mode we run all benchmarks matching filter/skip specified var benchmark: Benchmark? benchmarkCommand = .list diff --git a/Sources/Benchmark/Blackhole.swift b/Sources/Benchmark/Blackhole.swift index bd6bae62..37642626 100644 --- a/Sources/Benchmark/Blackhole.swift +++ b/Sources/Benchmark/Blackhole.swift @@ -29,10 +29,10 @@ /// } /// } /// ``` -@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf +@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf public func blackHole(_: some Any) {} -@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf +@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf public func identity(_ value: T) -> T { value } diff --git a/Sources/Benchmark/MallocStats/MallocStats.swift b/Sources/Benchmark/MallocStats/MallocStats.swift index 2056d413..14e1c10e 100644 --- a/Sources/Benchmark/MallocStats/MallocStats.swift +++ b/Sources/Benchmark/MallocStats/MallocStats.swift @@ -23,5 +23,5 @@ struct MallocStats { /// , and unused dirty pages. This is a maximum rather than precise because pages may /// not actually be physically resident if they correspond to demand-zeroed virtual memory /// that has not yet been touched. This is a multiple of the page size. - var allocatedResidentMemory: Int = 0 // in bytes + var allocatedResidentMemory: Int = 0 // in bytes } diff --git a/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift b/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift index ee35d299..d464d3ba 100644 --- a/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift +++ b/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift @@ -205,7 +205,7 @@ final class OperatingSystemStatsProducer { self.lock.unlock() - if firstEventSampled == false { // allow calling thread to continue when we have captured a sample + if firstEventSampled == false { // allow calling thread to continue when we have captured a sample firstEventSampled = true sampleSemaphore.signal() } diff --git a/Sources/Benchmark/Progress/ProgressElements.swift b/Sources/Benchmark/Progress/ProgressElements.swift index 26c4efa5..95920beb 100644 --- a/Sources/Benchmark/Progress/ProgressElements.swift +++ b/Sources/Benchmark/Progress/ProgressElements.swift @@ -83,7 +83,7 @@ public struct ProgressPercent: ProgressElementType { while padded.count < 4 { padded = " " + padded } - return padded // "\(percentDone.format(decimalPlaces))%" + return padded // "\(percentDone.format(decimalPlaces))%" } } diff --git a/Sources/Benchmark/Statistics.swift b/Sources/Benchmark/Statistics.swift index 49dec756..61ef3f25 100644 --- a/Sources/Benchmark/Statistics.swift +++ b/Sources/Benchmark/Statistics.swift @@ -15,18 +15,18 @@ import Numerics // A type that provides distribution / percentile calculations of latency measurements @_documentation(visibility: internal) public final class Statistics: Codable { - public static let defaultMaximumMeasurement = 1_000_000_000 // 1 second in nanoseconds + public static let defaultMaximumMeasurement = 1_000_000_000 // 1 second in nanoseconds public static let defaultPercentilesToCalculate = [0.0, 25.0, 50.0, 75.0, 90.0, 99.0, 100.0] public static let defaultPercentilesToCalculateP90Index = 4 public enum Units: Int, Codable, CaseIterable { - case count = 1 // e.g. nanoseconds - case kilo = 1_000 // microseconds - case mega = 1_000_000 // milliseconds - case giga = 1_000_000_000 // seconds - case tera = 1_000_000_000_000 // 1K seconds - case peta = 1_000_000_000_000_000 // 1M seconds - case automatic = 0 // will pick time unit above automatically + case count = 1 // e.g. nanoseconds + case kilo = 1_000 // microseconds + case mega = 1_000_000 // milliseconds + case giga = 1_000_000_000 // seconds + case tera = 1_000_000_000_000 // 1K seconds + case peta = 1_000_000_000_000_000 // 1M seconds + case automatic = 0 // will pick time unit above automatically public var description: String { switch self { diff --git a/Tests/BenchmarkTests/BenchmarkRunnerTests.swift b/Tests/BenchmarkTests/BenchmarkRunnerTests.swift index 31f17580..a7724d65 100644 --- a/Tests/BenchmarkTests/BenchmarkRunnerTests.swift +++ b/Tests/BenchmarkTests/BenchmarkRunnerTests.swift @@ -56,7 +56,7 @@ final class BenchmarkRunnerTests: XCTestCase, BenchmarkRunnerReadWrite { runner.quiet = false runner.timeUnits = .nanoseconds try await runner.run() - XCTAssertEqual(writeCount, 6) // 3 tests results + 3 end markers + XCTAssertEqual(writeCount, 6) // 3 tests results + 3 end markers } } From e5d361cf14a58eee4b4e09c0ae558d6b9cfe4463 Mon Sep 17 00:00:00 2001 From: MahdiBM Date: Mon, 14 Jul 2025 23:41:01 +0330 Subject: [PATCH 2/5] Implement relative and range thresholds from static files --- .../BenchmarkTool+Baselines.swift | 2 +- .../BenchmarkTool+Export+JMHFormatter.swift | 12 +- .../BenchmarkTool+Operations.swift | 6 +- .../BenchmarkTool+PrettyPrinting.swift | 143 +++++++------ ...chmarkTool+ReadP90AbsoluteThresholds.swift | 17 +- .../BenchmarkTool+Thresholds.swift | 95 ++++++--- Sources/Benchmark/BenchmarkMetric.swift | 12 ++ Sources/Benchmark/BenchmarkResult.swift | 193 +++++++++++++----- Sources/Benchmark/BenchmarkThresholds.swift | 172 ++++++++++++++++ Sources/Benchmark/Statistics.swift | 2 +- .../BenchmarkTests/BenchmarkResultTests.swift | 6 +- Tests/BenchmarkTests/StatisticsTests.swift | 2 +- 12 files changed, 501 insertions(+), 161 deletions(-) diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift b/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift index 40cdcaaf..c81aea90 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift @@ -482,7 +482,7 @@ extension BenchmarkBaseline: Equatable { public func failsAbsoluteThresholdChecks( benchmarks: [Benchmark], p90Thresholds: [BenchmarkIdentifier: - [BenchmarkMetric: BenchmarkThresholds.AbsoluteThreshold]] + [BenchmarkMetric: BenchmarkThreshold]] ) -> BenchmarkResult.ThresholdDeviations { var allDeviationResults = BenchmarkResult.ThresholdDeviations() diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift b/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift index 40d00caf..cd15b664 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift @@ -34,7 +34,7 @@ extension JMHPrimaryMetric { let factor = result.metric.countable == false ? 1_000 : 1 for p in percentiles { - percentileValues[String(p)] = Statistics.roundToDecimalplaces( + percentileValues[String(p)] = Statistics.roundToDecimalPlaces( Double(histogram.valueAtPercentile(p)) / Double(factor), 3 ) @@ -42,15 +42,15 @@ extension JMHPrimaryMetric { for value in histogram.recordedValues() { for _ in 0.. $1.uxPriority }) - let relativeResults = deviationResults.filter { - $0.name == nameAndTarget.name && $0.target == nameAndTarget.target && $0.metric == metric - && $0.relative == true - } - let absoluteResults = deviationResults.filter { - $0.name == nameAndTarget.name && $0.target == nameAndTarget.target && $0.metric == metric - && $0.relative == false - } let width = 40 - let percentileWidth = 15 + let percentileWidthFor4Columns = 15 + let percentileWidthFor3Columns = 20 + + let table = TextTable { + var columns: [Column] = [] + columns.reserveCapacity(4) + + let sign = + switch $0.deviation { + case .absolute: "Δ" + case .relative: "%" + case .range: "↔" + } + let unitDescription = metric.countable ? $0.units.description : $0.units.timeDescription + columns.append( + Column( + title: "\(metric.description) (\(unitDescription), \(sign))", + value: $0.percentile, + width: width, + align: .left + ) + ) - // The baseValue is the new baseline that we're using as the comparison base, so... - if absoluteResults.isEmpty == false { - let absoluteTable = TextTable { - [ - Column( - title: - "\(metric.description) (\(metric.countable ? $0.units.description : $0.units.timeDescription), Δ)", - value: $0.percentile, - width: width, - align: .left - ), + let baseValue = $0.baseValue + func baselineColumn(percentileWidth: Int) -> Column { + Column( + title: "\(comparingBaselineName)", + value: baseValue, + width: percentileWidth, + align: .right + ) + } + + // If absolute or relative add their columns together + var comparisonValue: Int? + var difference: String? + var tolerance: String? + switch $0.deviation { + case .absolute(let compareTo, let diff, let tol): + comparisonValue = compareTo + difference = diff.description + tolerance = tol.description + case .relative(let compareTo, let diff, let tol): + comparisonValue = compareTo + difference = Statistics.roundToDecimalPlaces(diff, 1).description + tolerance = Statistics.roundToDecimalPlaces(tol, 1).description + case .range: + break + } + + if let comparisonValue = comparisonValue, + let difference = difference, + let tolerance = tolerance + { + columns.append(contentsOf: [ Column( title: "\(baselineName)", - value: $0.comparisonValue, - width: percentileWidth, + value: comparisonValue, + width: percentileWidthFor4Columns, align: .right ), + baselineColumn(percentileWidth: percentileWidthFor4Columns), Column( - title: "\(comparingBaselineName)", - value: $0.baseValue, - width: percentileWidth, + title: "Difference \(sign)", + value: difference, + width: percentileWidthFor4Columns, align: .right ), - Column(title: "Difference Δ", value: $0.difference, width: percentileWidth, align: .right), Column( - title: "Threshold Δ", - value: $0.differenceThreshold, - width: percentileWidth, + title: "Tolerance \(sign)", + value: tolerance, + width: percentileWidthFor4Columns, align: .right ), - ] + ]) } - absoluteTable.print(absoluteResults, style: format.tableStyle) - } - - if relativeResults.isEmpty == false { - let relativeTable = TextTable { - [ - Column( - title: - "\(metric.description) (\(metric.countable ? $0.units.description : $0.units.timeDescription), %)", - value: $0.percentile, - width: width, - align: .left - ), + // Otherwise if range, then handle it alone + if case .range(let min, let max) = $0.deviation { + columns.append(contentsOf: [ + baselineColumn(percentileWidth: percentileWidthFor3Columns), Column( - title: "\(baselineName)", - value: $0.comparisonValue, - width: percentileWidth, + title: "Minimum", + value: min, + width: percentileWidthFor3Columns, align: .right ), Column( - title: "\(comparingBaselineName)", - value: $0.baseValue, - width: percentileWidth, + title: "Maximum", + value: max, + width: percentileWidthFor3Columns, align: .right ), - Column(title: "Difference %", value: $0.difference, width: percentileWidth, align: .right), - Column( - title: "Threshold %", - value: $0.differenceThreshold, - width: percentileWidth, - align: .right - ), - ] + ]) } - relativeTable.print(relativeResults, style: format.tableStyle) + return columns } + + table.print(filteredDeviations, style: format.tableStyle) } } } diff --git a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift index 066dd554..48a8faae 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift @@ -33,7 +33,7 @@ extension BenchmarkTool { static func makeBenchmarkThresholds( path: String, benchmarkIdentifier: BenchmarkIdentifier - ) -> [BenchmarkMetric: BenchmarkThresholds.AbsoluteThreshold]? { + ) -> [BenchmarkMetric: BenchmarkThreshold]? { var path = FilePath(path) if path.isAbsolute { path.append("\(benchmarkIdentifier.target).\(benchmarkIdentifier.name).p90.json") @@ -44,8 +44,7 @@ extension BenchmarkTool { path = cwdPath } - var p90Thresholds: [BenchmarkMetric: BenchmarkThresholds.AbsoluteThreshold] = [:] - var p90ThresholdsRaw: [String: BenchmarkThresholds.AbsoluteThreshold]? + var p90Thresholds: [BenchmarkMetric: BenchmarkThreshold] = [:] do { let fileDescriptor = try FileDescriptor.open(path, .readOnly, options: [], permissions: .ownerRead) @@ -66,19 +65,11 @@ extension BenchmarkTool { readBytes.append(contentsOf: nextBytes) } - p90ThresholdsRaw = try JSONDecoder() + p90Thresholds = try JSONDecoder() .decode( - [String: BenchmarkThresholds.AbsoluteThreshold].self, + [BenchmarkMetric: BenchmarkThreshold].self, from: Data(readBytes) ) - - if let p90ThresholdsRaw { - p90ThresholdsRaw.forEach { metric, threshold in - if let metric = BenchmarkMetric(argument: metric) { - p90Thresholds[metric] = threshold - } - } - } } catch { print( "Failed to read file at \(path) [\(String(reflecting: error))] \(Errno(rawValue: errno).description)" diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift b/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift index 3173b918..c343c39c 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift @@ -16,16 +16,17 @@ private let percentileWidth = 20 private let maxDescriptionWidth = 40 private struct ThresholdsTableEntry { + enum Values { + case absolute(p90: Int, absoluteTolerance: Int, relativeTolerance: Double) + case relativeOrRange(BenchmarkThreshold.RelativeOrRange) + } + var description: String - var p90: Int - var absolute: Int - var relative: Double + var value: Values } extension BenchmarkTool { - func printThresholds( - _ staticThresholdsPerBenchmark: [BenchmarkIdentifier: [BenchmarkMetric: BenchmarkThresholds.AbsoluteThreshold]] - ) { + func printThresholds(_ staticThresholdsPerBenchmark: [BenchmarkIdentifier: [BenchmarkMetric: BenchmarkThreshold]]) { guard !staticThresholdsPerBenchmark.isEmpty else { print("No thresholds defined.") @@ -35,13 +36,44 @@ extension BenchmarkTool { print("") var tableEntries: [ThresholdsTableEntry] = [] - let table = TextTable { - [ - Column(title: "Metric", value: "\($0.description)", width: maxDescriptionWidth, align: .left), - Column(title: "Threshold .p90", value: $0.p90, width: percentileWidth, align: .right), - Column(title: "Allowed %", value: $0.relative, width: percentileWidth, align: .right), - Column(title: "Allowed Δ", value: $0.absolute, width: percentileWidth, align: .right), - ] + let table = TextTable { entry in + var columns: [Column] = [] + columns.reserveCapacity(4) + + columns.append( + Column(title: "Metric", value: entry.description, width: maxDescriptionWidth, align: .left), + ) + + switch entry.value { + case .absolute(let p90, let absoluteTolerance, let relativeTolerance): + columns.append(contentsOf: [ + Column(title: "Threshold .p90", value: p90, width: percentileWidth, align: .right), + Column(title: "Allowed %", value: relativeTolerance, width: percentileWidth, align: .right), + Column(title: "Allowed Δ", value: absoluteTolerance, width: percentileWidth, align: .right), + ]) + case .relativeOrRange(let relativeOrRange): + relativeOrRange.preconditionContainsAnyValue() + + if let relative = relativeOrRange.relative { + let tolerancePercentage = Statistics.roundToDecimalPlaces(relative.tolerancePercentage, 1) + columns.append(contentsOf: [ + Column( + title: "Allowed %", + value: "\(relative.base) ± \(tolerancePercentage)%", + width: percentileWidth, + align: .right + ) + ]) + } + if let range = relativeOrRange.range { + columns.append(contentsOf: [ + Column(title: "Allowed min", value: "\(range.min)", width: percentileWidth, align: .right), + Column(title: "Allowed max", value: "\(range.max)", width: percentileWidth, align: .right), + ]) + } + } + + return columns } staticThresholdsPerBenchmark.forEach { benchmarkIdentifier, staticThresholds in @@ -50,25 +82,34 @@ extension BenchmarkTool { let thresholdDeviations = benchmarks.first(where: { benchmarkIdentifier - == .init( - target: $0.target, - name: $0.name - ) + == .init(target: $0.target, name: $0.name) })? .configuration.thresholds ?? .init() staticThresholds.forEach { threshold in - let absoluteThreshold = thresholdDeviations[threshold.key]?.absolute[.p90] ?? 0 - let relativeThreshold = thresholdDeviations[threshold.key]?.relative[.p90] ?? 0 - - tableEntries.append( - .init( - description: threshold.key.description, - p90: threshold.value, - absolute: absoluteThreshold, - relative: relativeThreshold + switch threshold.value { + case .absolute(let value): + let absoluteThreshold = thresholdDeviations[threshold.key]?.absolute[.p90] ?? 0 + let relativeThreshold = thresholdDeviations[threshold.key]?.relative[.p90] ?? 0 + + tableEntries.append( + .init( + description: threshold.key.description, + value: .absolute( + p90: value, + absoluteTolerance: absoluteThreshold, + relativeTolerance: relativeThreshold + ) + ) + ) + case .relativeOrRange(let relativeOrRange): + tableEntries.append( + .init( + description: threshold.key.description, + value: .relativeOrRange(relativeOrRange) + ) ) - ) + } } table.print(tableEntries, style: format.tableStyle) tableEntries = [] diff --git a/Sources/Benchmark/BenchmarkMetric.swift b/Sources/Benchmark/BenchmarkMetric.swift index 475f49f3..3a0edf9c 100644 --- a/Sources/Benchmark/BenchmarkMetric.swift +++ b/Sources/Benchmark/BenchmarkMetric.swift @@ -491,4 +491,16 @@ public extension BenchmarkMetric { } } +/// `CodingKeyRepresentable` conformance enables Codable to encode/decode a dictionary with keys of +/// type `BenchmarkMetric`, as if the key type was `String` and not `BenchmarkMetric`. +extension BenchmarkMetric: CodingKeyRepresentable { + public var codingKey: any CodingKey { + self.rawDescription.codingKey + } + + public init?(codingKey: T) where T: CodingKey { + self.init(argument: codingKey.stringValue) + } +} + // swiftlint:enable cyclomatic_complexity function_body_length diff --git a/Sources/Benchmark/BenchmarkResult.swift b/Sources/Benchmark/BenchmarkResult.swift index 47f19770..41d59c76 100644 --- a/Sources/Benchmark/BenchmarkResult.swift +++ b/Sources/Benchmark/BenchmarkResult.swift @@ -459,16 +459,43 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { } public struct ThresholdDeviation { + public enum Deviation { + case absolute(comparedTo: Int, difference: Int, tolerance: Int) + case relative(comparedTo: Int, differencePercentage: Double, tolerancePercentage: Double) + case range(min: Int, max: Int) + + public var isRelative: Bool { + switch self { + case .relative: + return true + default: + return false + } + } + + public var uxPriority: Int { + switch self { + case .range: + return 2 + case .absolute: + return 1 + case .relative: + return 0 + } + } + } + public let name: String public let target: String public let metric: BenchmarkMetric public let percentile: BenchmarkResult.Percentile public let baseValue: Int - public let comparisonValue: Int - public let difference: Int - public let differenceThreshold: Int - public let relative: Bool + public let deviation: Deviation public let units: Statistics.Units + + public var uxPriority: Int { + return deviation.uxPriority + } } public struct ThresholdDeviations { @@ -490,57 +517,127 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { func appendDeviationResultsFor( _ metric: BenchmarkMetric, _ lhs: Int, - _ rhs: Int, + /// Either absolute values or thresholds from static threshold files. + _ rhs: BenchmarkThreshold, _ percentile: Self.Percentile, - _ thresholds: BenchmarkThresholds, - _ scalingFactor: Statistics.Units, + /// Thresholds from 'configuration.thresholds' in the benchmark code. + _ thresholdsFromCode: BenchmarkThresholds, _ thresholdResults: inout ThresholdDeviations, _ name: String = "unknown name", _ target: String = "unknown target" ) { let reverseComparison = metric.polarity == .prefersLarger - let absoluteDifference = (reverseComparison ? -1 : 1) * (lhs - rhs) - let relativeDifference = - (reverseComparison ? 1 : -1) * (rhs != 0 ? (100 - (100.0 * Double(lhs) / Double(rhs))) : 0.0) - - if let threshold = thresholds.relative[percentile], !(-threshold...threshold).contains(relativeDifference) { - let deviation = ThresholdDeviation( - name: name, - target: target, - metric: metric, - percentile: percentile, - baseValue: normalize(lhs), - comparisonValue: normalize(rhs), - difference: Int(Statistics.roundToDecimalplaces(relativeDifference, 1)), - differenceThreshold: Int(threshold), - relative: true, - units: scalingFactor - ) - if relativeDifference > threshold { - thresholdResults.regressions.append(deviation) - } else if relativeDifference < -threshold { - thresholdResults.improvements.append(deviation) + switch rhs { + case .absolute(let rhs): + let absoluteDifference = (reverseComparison ? -1 : 1) * (lhs - rhs) + let relativeDifference = + (reverseComparison ? 1 : -1) * (rhs != 0 ? (100 - (100.0 * Double(lhs) / Double(rhs))) : 0.0) + + if let threshold = thresholdsFromCode.relative[percentile], + !(-threshold...threshold).contains(relativeDifference) + { + let deviation = ThresholdDeviation( + name: name, + target: target, + metric: metric, + percentile: percentile, + baseValue: normalize(lhs), + deviation: .relative( + comparedTo: normalize(rhs), + differencePercentage: Statistics.roundToDecimalPlaces(relativeDifference, 1), + tolerancePercentage: Statistics.roundToDecimalPlaces(threshold, 1) + ), + units: Statistics.Units(timeUnits) + ) + if relativeDifference > threshold { + thresholdResults.regressions.append(deviation) + } else if relativeDifference < -threshold { + thresholdResults.improvements.append(deviation) + } } - } - if let threshold = thresholds.absolute[percentile], !(-threshold...threshold).contains(absoluteDifference) { - let deviation = ThresholdDeviation( - name: name, - target: target, - metric: metric, - percentile: percentile, - baseValue: normalize(lhs), - comparisonValue: normalize(rhs), - difference: normalize(absoluteDifference), - differenceThreshold: normalize(threshold), - relative: false, - units: scalingFactor - ) + if let threshold = thresholdsFromCode.absolute[percentile], + !(-threshold...threshold).contains(absoluteDifference) + { + let deviation = ThresholdDeviation( + name: name, + target: target, + metric: metric, + percentile: percentile, + baseValue: normalize(lhs), + deviation: .absolute( + comparedTo: normalize(rhs), + difference: normalize(absoluteDifference), + tolerance: normalize(threshold) + ), + units: Statistics.Units(timeUnits) + ) + if absoluteDifference > threshold { + thresholdResults.regressions.append(deviation) + } else if absoluteDifference < -threshold { + thresholdResults.improvements.append(deviation) + } + } + + case .relativeOrRange(let rhs): + if thresholdsFromCode.definitelyContainsUserSpecifiedThresholds(at: percentile) { + print( + """ + Warning: Static threshold files contain relative or range thresholds for metric '\(metric)' at + percentile '\(percentile)', but 'configuration.thresholds' also contains threshold tolerance for this metric. + Will ignore 'configuration.thresholds'. + To silence this warning, remove 'configuration.thresholds' from your benchmark code of this benchmark. + + """ + ) + } + + if let relative = rhs.relative { + let relativeComparison = relative.contains(lhs) + if !relativeComparison.contains { + let relativeDifference = (reverseComparison ? 1 : -1) * relativeComparison.deviation + let deviation = ThresholdDeviation( + name: name, + target: target, + metric: metric, + percentile: percentile, + baseValue: normalize(lhs), + deviation: .relative( + comparedTo: normalize(relative.base), + differencePercentage: Statistics.roundToDecimalPlaces(relativeDifference, 1), + tolerancePercentage: Statistics.roundToDecimalPlaces( + relative.tolerancePercentage, + 1 + ) + ), + units: Statistics.Units(timeUnits) + ) + if relativeDifference > relative.tolerancePercentage { + thresholdResults.regressions.append(deviation) + } else if relativeDifference < -relative.tolerancePercentage { + thresholdResults.improvements.append(deviation) + } + } + } - if absoluteDifference > threshold { - thresholdResults.regressions.append(deviation) - } else if absoluteDifference < -threshold { - thresholdResults.improvements.append(deviation) + if let range = rhs.range, !range.contains(lhs) { + let deviation = ThresholdDeviation( + name: name, + target: target, + metric: metric, + percentile: percentile, + baseValue: normalize(lhs), + deviation: .range( + min: normalize(range.min), + max: normalize(range.max) + ), + units: Statistics.Units(timeUnits) + ) + if lhs < range.min { + thresholdResults.regressions.append(deviation) + } else if lhs > range.max { + thresholdResults.improvements.append(deviation) + } } } } @@ -565,7 +662,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { appendDeviationResultsFor( lhs.metric, lhsPercentiles[percentile], - rhsPercentiles[percentile], + .absolute(rhsPercentiles[percentile]), Self.Percentile(rawValue: percentile)!, thresholds, lhs.statistics.units(), @@ -581,7 +678,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { // Absolute checks for --check-absolute, just check p90 public func deviationsAgainstAbsoluteThresholds( thresholds: BenchmarkThresholds, - p90Threshold: BenchmarkThresholds.AbsoluteThreshold, + p90Threshold: BenchmarkThreshold, name: String = "test", target: String = "test" ) -> ThresholdDeviations { diff --git a/Sources/Benchmark/BenchmarkThresholds.swift b/Sources/Benchmark/BenchmarkThresholds.swift index 699df0ba..69db5d6c 100644 --- a/Sources/Benchmark/BenchmarkThresholds.swift +++ b/Sources/Benchmark/BenchmarkThresholds.swift @@ -32,3 +32,175 @@ public struct BenchmarkThresholds: Codable { public let relative: RelativeThresholds public let absolute: AbsoluteThresholds } + +extension BenchmarkThresholds { + public func definitelyContainsUserSpecifiedThresholds(at percentile: BenchmarkResult.Percentile) -> Bool { + let defaultCodeThresholds = BenchmarkThresholds.default + let relative = self.relative[percentile] + let absolute = self.absolute[percentile] + var relativeNonDefaultThresholdsExist: Bool { + (relative ?? 0) != 0 + && relative != defaultCodeThresholds.relative[percentile] + } + var absoluteNonDefaultThresholdsExist: Bool { + (absolute ?? 0) != 0 + && absolute != defaultCodeThresholds.absolute[percentile] + } + return relativeNonDefaultThresholdsExist || absoluteNonDefaultThresholdsExist + } +} + +public enum BenchmarkThreshold: Codable { + public struct RelativeOrRange: Codable { + public struct Relative: Encodable { + public let base: Int + public let tolerancePercentage: Double + + init(base: Int, tolerancePercentage: Double) { + precondition(base > 0, "base must be positive") + precondition(tolerancePercentage > 0, "tolerancePercentage must be positive") + self.base = base + self.tolerancePercentage = tolerancePercentage + } + + /// Returns whether or not the value satisfies this relative range, as well as the + /// percentage of the deviation of the value. + public func contains(_ value: Int) -> (contains: Bool, deviation: Double) { + let deviation = Double(value - base) / Double(base) * 100 + return (abs(deviation) <= tolerancePercentage, deviation) + } + } + + public struct Range: Encodable { + public let min: Int + public let max: Int + + init(min: Int, max: Int) { + precondition(min <= max, "min must be less than or equal to max") + self.min = min + self.max = max + } + + public func contains(_ value: Int) -> Bool { + return value >= min && value <= max + } + } + + public let relative: Relative? + public let range: Range? + + init(relative: Relative?, range: Range?) { + self.relative = relative + self.range = range + preconditionContainsAnyValue() + } + + public func preconditionContainsAnyValue() { + precondition( + self.containsAnyValue, + "RelativeOrRange must contain either a relative or range, but contains neither" + ) + } + + var containsAnyValue: Bool { + self.relative != nil || self.range != nil + } + + enum CodingKeys: String, CodingKey { + case base + case tolerancePercentage + case min + case max + } + + public init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + + let base = try container.decodeIfPresent(Int.self, forKey: .base) + let tolerancePercentage = try container.decodeIfPresent(Double.self, forKey: .tolerancePercentage) + let min = try container.decodeIfPresent(Int.self, forKey: .min) + let max = try container.decodeIfPresent(Int.self, forKey: .max) + + var relative: Relative? + var range: Range? + + if let base, let tolerancePercentage { + relative = Relative(base: base, tolerancePercentage: tolerancePercentage) + + guard base > 0, tolerancePercentage > 0 else { + throw DecodingError.dataCorrupted( + .init( + codingPath: decoder.codingPath, + debugDescription: """ + RelativeOrRange thresholds object contains an invalid relative values. + 'base' (\(base)) and 'tolerancePercentage' (\(tolerancePercentage)) must be positive. + """ + ) + ) + } + } + if let min, let max { + range = Range(min: min, max: max) + + guard min <= max else { + throw DecodingError.dataCorrupted( + .init( + codingPath: decoder.codingPath, + debugDescription: """ + RelativeOrRange thresholds object contains invalid min-max values. + 'min' (\(min)) and max ('\(max)') don't satisfy the requirements of min <= max. + """ + ) + ) + } + } + + self.relative = relative + self.range = range + + if !self.containsAnyValue { + throw DecodingError.dataCorrupted( + .init( + codingPath: decoder.codingPath, + debugDescription: """ + RelativeOrRange thresholds object does not contain either a valid relative or range. + For relative thresholds, both 'base' (Int) and 'tolerancePercentage' (Double) must be present and valid. + For range thresholds, both 'min' (Int) and 'max' (Int) must be present and valid. + You can declare both relative and range in the same object together, or just one of them. + Example: { "min": 90, "max": 110 } + Example: { "base": 115, "tolerancePercentage": 5.5 } + Example: { "base": 115, "tolerancePercentage": 4.5, "min": 90, "max": 110 } + """ + ) + ) + } + } + + public func encode(to encoder: any Encoder) throws { + try self.relative?.encode(to: encoder) + try self.range?.encode(to: encoder) + } + } + + case absolute(Int) + case relativeOrRange(RelativeOrRange) + + public init(from decoder: Decoder) throws { + let container = try decoder.singleValueContainer() + if let value = try? container.decode(Int.self) { + self = .absolute(value) + } else { + let value = try RelativeOrRange(from: decoder) + self = .relativeOrRange(value) + } + } + + public func encode(to encoder: Encoder) throws { + switch self { + case .absolute(let value): + try value.encode(to: encoder) + case .relativeOrRange(let value): + try value.encode(to: encoder) + } + } +} diff --git a/Sources/Benchmark/Statistics.swift b/Sources/Benchmark/Statistics.swift index 61ef3f25..6af433aa 100644 --- a/Sources/Benchmark/Statistics.swift +++ b/Sources/Benchmark/Statistics.swift @@ -181,7 +181,7 @@ public final class Statistics: Codable { } // Rounds decimals for display - public static func roundToDecimalplaces(_ original: Double, _ decimals: Int = 2) -> Double { + public static func roundToDecimalPlaces(_ original: Double, _ decimals: Int = 2) -> Double { let factor: Double = .pow(10.0, Double(decimals)) var original: Double = original * factor original.round(.toNearestOrEven) diff --git a/Tests/BenchmarkTests/BenchmarkResultTests.swift b/Tests/BenchmarkTests/BenchmarkResultTests.swift index 46170551..ee6cb29f 100644 --- a/Tests/BenchmarkTests/BenchmarkResultTests.swift +++ b/Tests/BenchmarkTests/BenchmarkResultTests.swift @@ -379,21 +379,21 @@ final class BenchmarkResultTests: XCTestCase { Benchmark.checkAbsoluteThresholds = true deviations = thirdResult.deviationsAgainstAbsoluteThresholds( thresholds: absoluteThresholdsP90, - p90Threshold: 1_497 + p90Threshold: .absolute(1_497) ) XCTAssertFalse(deviations.regressions.isEmpty) Benchmark.checkAbsoluteThresholds = true deviations = thirdResult.deviationsAgainstAbsoluteThresholds( thresholds: absoluteThresholdsP90, - p90Threshold: 1_505 + p90Threshold: .absolute(1_505) ) XCTAssertFalse(deviations.improvements.isEmpty) Benchmark.checkAbsoluteThresholds = true deviations = thirdResult.deviationsAgainstAbsoluteThresholds( thresholds: absoluteThresholdsP90, - p90Threshold: 1_501 + p90Threshold: .absolute(1_501) ) XCTAssertTrue(deviations.improvements.isEmpty && deviations.regressions.isEmpty) } diff --git a/Tests/BenchmarkTests/StatisticsTests.swift b/Tests/BenchmarkTests/StatisticsTests.swift index f330a15c..50d26c0b 100644 --- a/Tests/BenchmarkTests/StatisticsTests.swift +++ b/Tests/BenchmarkTests/StatisticsTests.swift @@ -26,7 +26,7 @@ final class StatisticsTests: XCTestCase { stats.add(measurement) } - XCTAssertEqual(Statistics.roundToDecimalplaces(123.4567898972239487234), 123.46) + XCTAssertEqual(Statistics.roundToDecimalPlaces(123.4567898972239487234), 123.46) XCTAssertEqual(stats.measurementCount, measurementCount * 2) XCTAssertEqual(stats.units(), .count) XCTAssertEqual(round(stats.histogram.mean), round(Double(measurementCount / 2))) From 4729c14d31dbc75c2434d60c00f0c50293f1128c Mon Sep 17 00:00:00 2001 From: MahdiBM Date: Tue, 15 Jul 2025 12:06:48 +0330 Subject: [PATCH 3/5] Add --skip-loading-benchmark-targets flag --- .../BenchmarkCommandPlugin.swift | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift index ac3657d6..f1e95cd3 100644 --- a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift +++ b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift @@ -41,6 +41,7 @@ import Glibc let quietRunning = argumentExtractor.extractFlag(named: "quiet") let noProgress = argumentExtractor.extractFlag(named: "no-progress") let checkAbsoluteThresholdsPath = argumentExtractor.extractOption(named: "check-absolute-path") + let skipLoadingBenchmarks = argumentExtractor.extractFlag(named: "skip-loading-benchmark-targets") let checkAbsoluteThresholds = checkAbsoluteThresholdsPath.count > 0 ? 1 : argumentExtractor.extractFlag(named: "check-absolute") let groupingToUse = argumentExtractor.extractOption(named: "grouping") @@ -233,6 +234,7 @@ import Glibc throw MyError.invalidArgument } + var skipLoadingBenchmarksFlagIsValid = skipLoadingBenchmarks == 0 if commandToPerform == .thresholds { guard positionalArguments.count > 0, let thresholdsOperation = ThresholdsOperation(rawValue: positionalArguments.removeFirst()) @@ -267,6 +269,8 @@ import Glibc } break case .check: + skipLoadingBenchmarksFlagIsValid = true + shouldBuildTargets = skipLoadingBenchmarks == 0 let validRange = 0...1 guard validRange.contains(positionalArguments.count) else { print( @@ -281,6 +285,19 @@ import Glibc } } + if !skipLoadingBenchmarksFlagIsValid { + print("") + print( + "Flag --skip-loading-benchmark-targets is only valid for 'thresholds check' operations." + ) + print("") + print(help) + print("") + print("Please visit https://github.com/ordo-one/package-benchmark for more in-depth documentation") + print("") + throw MyError.invalidArgument + } + if commandToPerform == .baseline { guard positionalArguments.count > 0, let baselineOperation = BaselineOperation(rawValue: positionalArguments.removeFirst()) From 2e1b93cff7047ec9a004b49c46a2ff4c3ef60494 Mon Sep 17 00:00:00 2001 From: MahdiBM Date: Tue, 15 Jul 2025 18:17:41 +0330 Subject: [PATCH 4/5] Implement running 'benchmark thresholds update' multiple times --- .../BenchmarkCommandPlugin.swift | 167 +++++++++++++----- .../BenchmarkHelpGenerator.swift | 36 ++++ .../BenchmarkTool/BenchmarkTool+Export.swift | 158 +++++++++++++++-- .../BenchmarkTool+Operations.swift | 7 +- .../BenchmarkTool+PrettyPrinting.swift | 131 ++++++-------- ...chmarkTool+ReadP90AbsoluteThresholds.swift | 16 ++ .../BenchmarkTool+Thresholds.swift | 4 +- Plugins/BenchmarkTool/BenchmarkTool.swift | 12 ++ Sources/Benchmark/BenchmarkResult.swift | 82 +++++---- Sources/Benchmark/BenchmarkThresholds.swift | 96 +++++++--- Sources/Benchmark/Statistics.swift | 8 +- 11 files changed, 517 insertions(+), 200 deletions(-) diff --git a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift index f1e95cd3..b866b8cd 100644 --- a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift +++ b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift @@ -44,6 +44,9 @@ import Glibc let skipLoadingBenchmarks = argumentExtractor.extractFlag(named: "skip-loading-benchmark-targets") let checkAbsoluteThresholds = checkAbsoluteThresholdsPath.count > 0 ? 1 : argumentExtractor.extractFlag(named: "check-absolute") + let runCount = argumentExtractor.extractOption(named: "run-count") + let relative = argumentExtractor.extractFlag(named: "relative") + let range = argumentExtractor.extractFlag(named: "range") let groupingToUse = argumentExtractor.extractOption(named: "grouping") let metricsToUse = argumentExtractor.extractOption(named: "metric") let timeUnits = argumentExtractor.extractOption(named: "time-units") @@ -234,6 +237,7 @@ import Glibc throw MyError.invalidArgument } + var totalRunCount = 1 var skipLoadingBenchmarksFlagIsValid = skipLoadingBenchmarks == 0 if commandToPerform == .thresholds { guard positionalArguments.count > 0, @@ -264,10 +268,27 @@ import Glibc ) throw MyError.invalidArgument } - if positionalArguments.count > 0 { + let usesExistingBaseline = positionalArguments.count > 0 + if usesExistingBaseline { shouldBuildTargets = false } - break + let requestedRunCount = runCount.first.flatMap { Int($0) } ?? 1 + /// These update the run count to 5 by default if it's set to 1. + /// Using relative/range flags doesn't mean anything if we're not running multiple times. + /// The benchmarks will need to be run multiple times in order to be able to calculate a + /// relative/range of thresholds which satisfy all benchmark runs. + if relative > 0 { + args.append("--wants-relative-thresholds") + if !usesExistingBaseline { + totalRunCount = requestedRunCount < 2 ? 5 : requestedRunCount + } + } + if range > 0 { + args.append("--wants-range-thresholds") + if !usesExistingBaseline { + totalRunCount = requestedRunCount < 2 ? 5 : requestedRunCount + } + } case .check: skipLoadingBenchmarksFlagIsValid = true shouldBuildTargets = skipLoadingBenchmarks == 0 @@ -480,53 +501,108 @@ import Glibc var failedBenchmarkCount = 0 - try withCStrings(args) { cArgs in - if debug > 0 { - print("To debug, start \(benchmarkToolName) in LLDB using:") - print("lldb \(benchmarkTool.string)") - print("") - print("Then launch \(benchmarkToolName) with:") - print("run \(args.dropFirst().joined(separator: " "))") - print("") - return - } + var allFailureCount = 0 + let results: [Result] = (0.. 1 { + args += ["--run-number", "\(runIdx + 1)"] + if quietRunning == 0 { + print( + """ - var pid: pid_t = 0 - var status = posix_spawn(&pid, benchmarkTool.string, nil, nil, cArgs, environ) - - if status == 0 { - if waitpid(pid, &status, 0) != -1 { - // Ok, this sucks, but there is no way to get a C support target for plugins and - // the way the status is extracted portably is with macros - so we just need to - // reimplement the logic here in Swift according to the waitpid man page to - // get some nicer feedback on failure reason. - guard let waitStatus = ExitCode(rawValue: (status & 0xFF00) >> 8) else { - print("One or more benchmarks returned an unexpected return code \(status)") - throw MyError.benchmarkUnexpectedReturnCode + Running the command multiple times, round \(runIdx + 1) of \(totalRunCount)... + """ + ) } - switch waitStatus { - case .success: - break - case .baselineNotFound: - throw MyError.baselineNotFound - case .genericFailure: - print("One or more benchmark suites crashed during runtime.") - throw MyError.benchmarkCrashed - case .thresholdRegression: - throw MyError.benchmarkThresholdRegression - case .thresholdImprovement: - throw MyError.benchmarkThresholdImprovement - case .benchmarkJobFailed: - failedBenchmarkCount += 1 - case .noPermissions: - throw MyError.noPermissions + } + + return Result { + try withCStrings(args) { cArgs in + /// We'll decrement this in the success path + allFailureCount += 1 + + if debug > 0 { + print("To debug, start \(benchmarkToolName) in LLDB using:") + print("lldb \(benchmarkTool.string)") + print("") + print("Then launch \(benchmarkToolName) with:") + print("run \(args.dropFirst().joined(separator: " "))") + print("") + return + } + + var pid: pid_t = 0 + var status = posix_spawn(&pid, benchmarkTool.string, nil, nil, cArgs, environ) + + if status == 0 { + if waitpid(pid, &status, 0) != -1 { + // Ok, this sucks, but there is no way to get a C support target for plugins and + // the way the status is extracted portably is with macros - so we just need to + // reimplement the logic here in Swift according to the waitpid man page to + // get some nicer feedback on failure reason. + guard let waitStatus = ExitCode(rawValue: (status & 0xFF00) >> 8) else { + print("One or more benchmarks returned an unexpected return code \(status)") + throw MyError.benchmarkUnexpectedReturnCode + } + switch waitStatus { + case .success: + allFailureCount -= 1 + case .baselineNotFound: + throw MyError.baselineNotFound + case .genericFailure: + print("One or more benchmark suites crashed during runtime.") + throw MyError.benchmarkCrashed + case .thresholdRegression: + throw MyError.benchmarkThresholdRegression + case .thresholdImprovement: + throw MyError.benchmarkThresholdImprovement + case .benchmarkJobFailed: + failedBenchmarkCount += 1 + case .noPermissions: + throw MyError.noPermissions + } + } else { + print( + "waitpid() for pid \(pid) returned a non-zero exit code \(status), errno = \(errno)" + ) + exit(errno) + } + } else { + print("Failed to run BenchmarkTool, posix_spawn() returned [\(status)]") + } } - } else { - print("waitpid() for pid \(pid) returned a non-zero exit code \(status), errno = \(errno)") - exit(errno) } - } else { - print("Failed to run BenchmarkTool, posix_spawn() returned [\(status)]") + } + + switch results.count { + case ...0: + throw MyError.unknownFailure + case 1: + try results[0].get() + default: + if allFailureCount > 0 { + print( + """ + Ran BenchmarkTool \(results.count) times, but it failed \(allFailureCount) times. + Will exit with the first failure. + + """ + ) + guard + let failure = results.first(where: { result in + switch result { + case .failure: + return true + case .success: + return false + } + }) + else { + throw MyError.unknownFailure + } + try failure.get() } } @@ -546,5 +622,6 @@ import Glibc case noPermissions = 6 case invalidArgument = 101 case buildFailed = 102 + case unknownFailure = 103 } } diff --git a/Plugins/BenchmarkHelpGenerator/BenchmarkHelpGenerator.swift b/Plugins/BenchmarkHelpGenerator/BenchmarkHelpGenerator.swift index 166ea313..8b5902d8 100644 --- a/Plugins/BenchmarkHelpGenerator/BenchmarkHelpGenerator.swift +++ b/Plugins/BenchmarkHelpGenerator/BenchmarkHelpGenerator.swift @@ -153,6 +153,42 @@ struct Benchmark: AsyncParsableCommand { ) var checkAbsolute = false + @Flag( + name: .long, + help: """ + Specifies that thresholds check command should skip loading benchmark targets. + Use this flag to skip unnecessary building of benchmark targets and loading of benchmark results, to save time. + This flag is specially useful when combined with static threshold files that contain the newly supported relative or range thresholds. + With such a set up, you'll save the time needed to build the benchmark targets and the thresholds check operation + will only read the threshold tolerance values from the static files. + """ + ) + var skipLoadingBenchmarks = false + + @Option( + name: .long, + help: """ + The number of times to run each benchmark in thresholds update operation. + This is only valid when --relative or --range are also specified. + When combined with --relative or --range flags, this option will run the benchmarks multiple times to calculate + relative or range thresholds, and each time it'll widen the threshold tolerances according to the new result. + Defaults to 1. + """ + ) + var runCount: Int? + + @Flag( + name: .long, + help: "Specifies that thresholds update command should output relative thresholds to the static files." + ) + var relative = false + + @Flag( + name: .long, + help: "Specifies that thresholds update command should output min-max range thresholds to the static files." + ) + var range = false + @Option( name: .long, help: diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Export.swift b/Plugins/BenchmarkTool/BenchmarkTool+Export.swift index 48802577..c0d6dad8 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Export.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Export.swift @@ -21,18 +21,17 @@ import Glibc #endif extension BenchmarkTool { - func write( - exportData: String, - hostIdentifier: String? = nil, - fileName: String = "results.txt" - ) throws { - // Set up desired output path and create any intermediate directories for structure as required: + enum OutputPath { + case stdout + case file(FilePath) + } + + func outputPath(hostIdentifier: String? = nil, fileName: String) -> OutputPath { var outputPath: FilePath if let path = (thresholdsOperation == nil) ? path : thresholdsPath { if path == "stdout" { - print(exportData) - return + return .stdout } let subPath = FilePath(path).removingRoot() @@ -57,6 +56,24 @@ extension BenchmarkTool { outputPath.append(csvFile.components) + return .file(outputPath) + } + + func write( + exportData: String, + hostIdentifier: String? = nil, + fileName: String = "results.txt" + ) throws { + // Set up desired output path and create any intermediate directories for structure as required: + let outputPath: FilePath + switch self.outputPath(hostIdentifier: hostIdentifier, fileName: fileName) { + case .stdout: + print(exportData) + return + case .file(let path): + outputPath = path + } + print("Writing to \(outputPath)") printFailedBenchmarks() @@ -254,15 +271,124 @@ extension BenchmarkTool { } } case .metricP90AbsoluteThresholds: + let jsonEncoder = JSONEncoder() + jsonEncoder.outputFormatting = [.prettyPrinted, .sortedKeys] + try baseline.results.forEach { key, results in - let jsonEncoder = JSONEncoder() - jsonEncoder.outputFormatting = [.prettyPrinted, .sortedKeys] + let fileName = cleanupStringForShellSafety("\(key.target).\(key.name).p90.json") + + var outputResults: [BenchmarkMetric: BenchmarkThreshold] = [:] + + let wantsRelative = self.wantsRelativeThresholds + let wantsRange = self.wantsRangeThresholds + let wantsRelativeOrRange = wantsRelative || wantsRange + + /// If it's the first run or if relative/range are not specified, then + /// override the thresholds file with the new results we have. + /// If runNumber is zero that'd mean this is not part of a multi-run benchmark, + /// so we'll still try to update thresholds instead of overriding them. + if runNumber == 1 || !wantsRelativeOrRange { + for values in results { + outputResults[values.metric] = .absolute( + Int(values.statistics.histogram.valueAtPercentile(90.0)) + ) + } - var outputResults: [String: BenchmarkThresholds.AbsoluteThreshold] = [:] - results.forEach { values in - outputResults[values.metric.rawDescription] = Int( - values.statistics.histogram.valueAtPercentile(90.0) - ) + } else { + /// If it's not the first run and any of relative/range are specified, then + /// merge the new results with the existing thresholds. + + var currentThresholds: [BenchmarkMetric: BenchmarkThreshold]? + + switch self.outputPath(fileName: fileName) { + case .stdout: + currentThresholds = nil + case .file(let path): + currentThresholds = Self.makeBenchmarkThresholds( + path: path, + benchmarkIdentifier: key + ) + } + + outputResults = currentThresholds ?? [:] + + for values in results { + let metric = values.metric + let newValue = values.statistics.histogram.valueAtPercentile(90.0) + + var relativeResult: BenchmarkThreshold.RelativeOrRange.Relative? + var rangeResult: BenchmarkThreshold.RelativeOrRange.Range? + if wantsRelativeOrRange { + let newValue = Double(Int(truncatingIfNeeded: newValue)) + /// Prefer Double to keep precision + var min = Double(newValue) + var max = Double(newValue) + + /// Load current min/max values from static thresholds file + switch currentThresholds?[metric] { + case .absolute(let value): + min = Double(value) + max = Double(value) + case .relativeOrRange(let relativeOrRange): + /// If for "wantsRelative", we prefer to use the min/max + if let range = relativeOrRange.range { + min = Double(range.min) + max = Double(range.max) + } else if let relative = relativeOrRange.relative { + let base = Double(relative.base) + let diff = (base / 100) * relative.tolerancePercentage + min = base - diff + max = base + diff + } + case .none: break + } + + /// Update the min/max values + min = Swift.min(min, Double(newValue)) + max = Swift.max(max, Double(newValue)) + + /// If min == max, it won't make a difference than using .absolute + if min != max { + if wantsRange { + rangeResult = .init(min: Int(min), max: Int(max)) + } + + if wantsRelative { + /// Calculate base and tolerancePercentage + let base = (min + max) / 2 + let diff = max - base + let diffPercentage = (base == 0) ? 0 : (diff / base * 100) + let tolerancePercentage = Statistics.roundToDecimalPlaces(diffPercentage, 2, .up) + + relativeResult = .init( + base: Int(base), + tolerancePercentage: tolerancePercentage + ) + } + } + } + + if relativeResult == nil && rangeResult == nil { + outputResults[metric] = .absolute(Int(truncatingIfNeeded: newValue)) + } else { + /// If we have a relative/range threshold but it's not specified in the command for + /// this run to update it, we still would like to keep the non-updated existing threshold. + switch currentThresholds?[metric] { + case .relativeOrRange(let currentRelativeOrRange): + relativeResult = relativeResult ?? currentRelativeOrRange.relative + rangeResult = rangeResult ?? currentRelativeOrRange.range + case .absolute, .none: + break + } + + outputResults[metric] = .relativeOrRange( + BenchmarkThreshold.RelativeOrRange( + relative: relativeResult, + range: rangeResult + ) + ) + } + } } let jsonResultData = try jsonEncoder.encode(outputResults) @@ -270,7 +396,7 @@ extension BenchmarkTool { if let stringOutput = String(data: jsonResultData, encoding: .utf8) { try write( exportData: stringOutput, - fileName: cleanupStringForShellSafety("\(key.target).\(key.name).p90.json") + fileName: fileName ) } else { print("Failed to encode json for \(outputResults)") diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift b/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift index 1824dc04..86b243e6 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift @@ -79,12 +79,17 @@ extension BenchmarkTool { return cleanedString } - struct NameAndTarget: Hashable { + struct NameAndTarget: Hashable, Comparable { let name: String let target: String + + static func < (lhs: NameAndTarget, rhs: NameAndTarget) -> Bool { + (lhs.target, lhs.name) < (rhs.target, rhs.name) + } } mutating func postProcessBenchmarkResults() throws { + // Turn on buffering again for output setvbuf(stdout, nil, _IOFBF, Int(BUFSIZ)) diff --git a/Plugins/BenchmarkTool/BenchmarkTool+PrettyPrinting.swift b/Plugins/BenchmarkTool/BenchmarkTool+PrettyPrinting.swift index ac48e1a5..29dc39ee 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+PrettyPrinting.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+PrettyPrinting.swift @@ -536,28 +536,25 @@ extension BenchmarkTool { guard quiet == false else { return } let metrics = deviationResults.map(\.metric).unique() - // Get a unique set of all name/target pairs that have threshold deviations, sorted lexically: - let namesAndTargets = deviationResults.map { NameAndTarget(name: $0.name, target: $0.target) } - .unique().sorted { ($0.target, $0.name) < ($1.target, $1.name) } - namesAndTargets.forEach { nameAndTarget in + var groupedDeviations = [NameAndTarget: [BenchmarkResult.ThresholdDeviation]]() + groupedDeviations.reserveCapacity(deviationResults.count) + for result in deviationResults { + let nameAndTarget = NameAndTarget(name: result.name, target: result.target) + groupedDeviations[nameAndTarget, default: []].append(result) + } + let sortedGroupedDeviations = groupedDeviations.sorted(by: { $0.key < $1.key }) + for (nameAndTarget, deviationResults) in sortedGroupedDeviations { printMarkdown("```") "\(deviationTitle) for \(nameAndTarget.target):\(nameAndTarget.name)".printAsHeader(addWhiteSpace: false) printMarkdown("```") metrics.forEach { metric in - let filteredDeviations = - deviationResults.filter { - $0.name == nameAndTarget.name - && $0.target == nameAndTarget.target - && $0.metric == metric - } - .sorted(by: { $0.uxPriority > $1.uxPriority }) + let filteredDeviations = deviationResults.filter { $0.metric == metric } let width = 40 - let percentileWidthFor4Columns = 15 - let percentileWidthFor3Columns = 20 + let percentileWidth = 15 let table = TextTable { var columns: [Column] = [] @@ -569,6 +566,11 @@ extension BenchmarkTool { case .relative: "%" case .range: "↔" } + let diffSign = + switch $0.deviation { + case .absolute, .range: "Δ" + case .relative: "%" + } let unitDescription = metric.countable ? $0.units.description : $0.units.timeDescription columns.append( Column( @@ -580,82 +582,65 @@ extension BenchmarkTool { ) let baseValue = $0.baseValue - func baselineColumn(percentileWidth: Int) -> Column { - Column( - title: "\(comparingBaselineName)", - value: baseValue, - width: percentileWidth, - align: .right - ) - } - // If absolute or relative add their columns together - var comparisonValue: Int? - var difference: String? - var tolerance: String? + // If absolute or relative, we can calculate their columns together + let comparisonValue: String + let difference: String + let tolerance: String? switch $0.deviation { case .absolute(let compareTo, let diff, let tol): - comparisonValue = compareTo + comparisonValue = "\(compareTo)" difference = diff.description tolerance = tol.description case .relative(let compareTo, let diff, let tol): - comparisonValue = compareTo - difference = Statistics.roundToDecimalPlaces(diff, 1).description - tolerance = Statistics.roundToDecimalPlaces(tol, 1).description - case .range: - break + comparisonValue = "\(compareTo)" + difference = Statistics.roundToDecimalPlaces(diff, 2).description + tolerance = Statistics.roundToDecimalPlaces(tol, 2).description + case .range(let min, let max): + comparisonValue = "\(min) ... \(max)" + let diff = baseValue >= max ? baseValue - max : baseValue - min + difference = "\(diff)" + tolerance = nil } - if let comparisonValue = comparisonValue, - let difference = difference, - let tolerance = tolerance - { - columns.append(contentsOf: [ - Column( - title: "\(baselineName)", - value: comparisonValue, - width: percentileWidthFor4Columns, - align: .right - ), - baselineColumn(percentileWidth: percentileWidthFor4Columns), - Column( - title: "Difference \(sign)", - value: difference, - width: percentileWidthFor4Columns, - align: .right - ), - Column( - title: "Tolerance \(sign)", - value: tolerance, - width: percentileWidthFor4Columns, - align: .right - ), - ]) - } + columns.append(contentsOf: [ + Column( + title: baselineName, + value: comparisonValue, + width: percentileWidth, + align: .right + ), + Column( + title: comparingBaselineName, + value: baseValue, + width: percentileWidth, + align: .right + ), + Column( + title: "Difference \(diffSign)", + value: difference, + width: percentileWidth, + align: .right + ), + ]) - // Otherwise if range, then handle it alone - if case .range(let min, let max) = $0.deviation { - columns.append(contentsOf: [ - baselineColumn(percentileWidth: percentileWidthFor3Columns), - Column( - title: "Minimum", - value: min, - width: percentileWidthFor3Columns, - align: .right - ), + if let tolerance = tolerance { + columns.append( Column( - title: "Maximum", - value: max, - width: percentileWidthFor3Columns, + title: "Tolerance \(diffSign)", + value: tolerance, + width: percentileWidth, align: .right - ), - ]) + ) + ) } return columns } - table.print(filteredDeviations, style: format.tableStyle) + table.print(filteredDeviations.filter(\.deviation.isRange), style: format.tableStyle) + table.print(filteredDeviations.filter(\.deviation.isAbsolute), style: format.tableStyle) + table.print(filteredDeviations.filter(\.deviation.isRelative), style: format.tableStyle) } } } diff --git a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift index 48a8faae..f40a4d46 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift @@ -44,6 +44,22 @@ extension BenchmarkTool { path = cwdPath } + return makeBenchmarkThresholds(path: path, benchmarkIdentifier: benchmarkIdentifier) + } + + /// `makeBenchmarkThresholds` is a convenience function for reading p90 static thresholds that previously have been exported with `metricP90AbsoluteThresholds` + /// + /// - Parameters: + /// - path: The path where the `Thresholds` directory should be located, containing static thresholds files using the naming pattern: + /// `moduleName.benchmarkName.p90.json` + /// - moduleName: The name of the benchmark module, can be extracted in the benchmark using: + /// `String("\(#fileID)".prefix(while: { $0 != "/" }))` + /// - benchmarkName: The name of the benchmark + /// - Returns: A dictionary with static benchmark thresholds per metric or nil if the file could not be found or read + static func makeBenchmarkThresholds( + path: FilePath, + benchmarkIdentifier: BenchmarkIdentifier + ) -> [BenchmarkMetric: BenchmarkThreshold]? { var p90Thresholds: [BenchmarkMetric: BenchmarkThreshold] = [:] do { diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift b/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift index c343c39c..2d2d0648 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Thresholds.swift @@ -52,10 +52,8 @@ extension BenchmarkTool { Column(title: "Allowed Δ", value: absoluteTolerance, width: percentileWidth, align: .right), ]) case .relativeOrRange(let relativeOrRange): - relativeOrRange.preconditionContainsAnyValue() - if let relative = relativeOrRange.relative { - let tolerancePercentage = Statistics.roundToDecimalPlaces(relative.tolerancePercentage, 1) + let tolerancePercentage = Statistics.roundToDecimalPlaces(relative.tolerancePercentage, 2) columns.append(contentsOf: [ Column( title: "Allowed %", diff --git a/Plugins/BenchmarkTool/BenchmarkTool.swift b/Plugins/BenchmarkTool/BenchmarkTool.swift index 6bb6233c..974a68f3 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool.swift @@ -71,6 +71,15 @@ struct BenchmarkTool: AsyncParsableCommand { @Option(name: .long, help: "The operation to perform for thresholds") var thresholdsOperation: ThresholdsOperation? + @Option(name: .long, help: "The run number of the benchmark tool. Defaults to 1.") + var runNumber: Int? + + @Flag(name: .long, help: "True if we should add relative thresholds to the static files in thresholds update.") + var wantsRelativeThresholds = false + + @Flag(name: .long, help: "True if we should add min-max range thresholds to the static files in thresholds update.") + var wantsRangeThresholds = false + @Flag(name: .long, help: "True if we should suppress output") var quiet: Bool = false @@ -232,6 +241,9 @@ struct BenchmarkTool: AsyncParsableCommand { return } + // Make sure this never goes below 1 + self.runNumber = runNumber.map { max($0, 1) } + // Skip reading baselines for baseline operations not needing them if let operation = baselineOperation, [.delete, .list, .update].contains(operation) == false { try readBaselines() diff --git a/Sources/Benchmark/BenchmarkResult.swift b/Sources/Benchmark/BenchmarkResult.swift index 41d59c76..5b0fd5cd 100644 --- a/Sources/Benchmark/BenchmarkResult.swift +++ b/Sources/Benchmark/BenchmarkResult.swift @@ -468,19 +468,26 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { switch self { case .relative: return true - default: + case .absolute, .range: return false } } - public var uxPriority: Int { + public var isRange: Bool { switch self { case .range: - return 2 + return true + case .absolute, .relative: + return false + } + } + + public var isAbsolute: Bool { + switch self { case .absolute: - return 1 - case .relative: - return 0 + return true + case .relative, .range: + return false } } } @@ -492,10 +499,6 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { public let baseValue: Int public let deviation: Deviation public let units: Statistics.Units - - public var uxPriority: Int { - return deviation.uxPriority - } } public struct ThresholdDeviations { @@ -526,12 +529,25 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { _ name: String = "unknown name", _ target: String = "unknown target" ) { - let reverseComparison = metric.polarity == .prefersLarger + + let needsReverseComparison = metric.polarity == .prefersLarger + /// By default, exceeding a threshold is a regression. + /// If `needsReverseComparison` then exceeding a threshold is an improvement. + func keyPathForAppending( + exceedsThreshold: Bool + ) -> WritableKeyPath { + switch exceedsThreshold { + case true: + return needsReverseComparison ? \.improvements : \.regressions + case false: + return needsReverseComparison ? \.regressions : \.improvements + } + } + switch rhs { case .absolute(let rhs): - let absoluteDifference = (reverseComparison ? -1 : 1) * (lhs - rhs) - let relativeDifference = - (reverseComparison ? 1 : -1) * (rhs != 0 ? (100 - (100.0 * Double(lhs) / Double(rhs))) : 0.0) + let absoluteDifference = lhs - rhs + let relativeDifference = (rhs != 0 ? (Double(absoluteDifference) / Double(rhs)) : 0.0) if let threshold = thresholdsFromCode.relative[percentile], !(-threshold...threshold).contains(relativeDifference) @@ -544,16 +560,13 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { baseValue: normalize(lhs), deviation: .relative( comparedTo: normalize(rhs), - differencePercentage: Statistics.roundToDecimalPlaces(relativeDifference, 1), - tolerancePercentage: Statistics.roundToDecimalPlaces(threshold, 1) + differencePercentage: Statistics.roundToDecimalPlaces(relativeDifference, 2), + tolerancePercentage: Statistics.roundToDecimalPlaces(threshold, 2) ), units: Statistics.Units(timeUnits) ) - if relativeDifference > threshold { - thresholdResults.regressions.append(deviation) - } else if relativeDifference < -threshold { - thresholdResults.improvements.append(deviation) - } + let keyPath = keyPathForAppending(exceedsThreshold: relativeDifference > threshold) + thresholdResults[keyPath: keyPath].append(deviation) } if let threshold = thresholdsFromCode.absolute[percentile], @@ -572,11 +585,8 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { ), units: Statistics.Units(timeUnits) ) - if absoluteDifference > threshold { - thresholdResults.regressions.append(deviation) - } else if absoluteDifference < -threshold { - thresholdResults.improvements.append(deviation) - } + let keyPath = keyPathForAppending(exceedsThreshold: absoluteDifference > threshold) + thresholdResults[keyPath: keyPath].append(deviation) } case .relativeOrRange(let rhs): @@ -595,7 +605,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { if let relative = rhs.relative { let relativeComparison = relative.contains(lhs) if !relativeComparison.contains { - let relativeDifference = (reverseComparison ? 1 : -1) * relativeComparison.deviation + let relativeDifference = relativeComparison.deviation let deviation = ThresholdDeviation( name: name, target: target, @@ -604,19 +614,16 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { baseValue: normalize(lhs), deviation: .relative( comparedTo: normalize(relative.base), - differencePercentage: Statistics.roundToDecimalPlaces(relativeDifference, 1), + differencePercentage: Statistics.roundToDecimalPlaces(relativeDifference, 2), tolerancePercentage: Statistics.roundToDecimalPlaces( relative.tolerancePercentage, - 1 + 2 ) ), units: Statistics.Units(timeUnits) ) - if relativeDifference > relative.tolerancePercentage { - thresholdResults.regressions.append(deviation) - } else if relativeDifference < -relative.tolerancePercentage { - thresholdResults.improvements.append(deviation) - } + let keyPath = keyPathForAppending(exceedsThreshold: relativeDifference.sign == .plus) + thresholdResults[keyPath: keyPath].append(deviation) } } @@ -633,11 +640,8 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { ), units: Statistics.Units(timeUnits) ) - if lhs < range.min { - thresholdResults.regressions.append(deviation) - } else if lhs > range.max { - thresholdResults.improvements.append(deviation) - } + let keyPath = keyPathForAppending(exceedsThreshold: lhs > range.max) + thresholdResults[keyPath: keyPath].append(deviation) } } } diff --git a/Sources/Benchmark/BenchmarkThresholds.swift b/Sources/Benchmark/BenchmarkThresholds.swift index 69db5d6c..a00b049b 100644 --- a/Sources/Benchmark/BenchmarkThresholds.swift +++ b/Sources/Benchmark/BenchmarkThresholds.swift @@ -51,55 +51,97 @@ extension BenchmarkThresholds { } public enum BenchmarkThreshold: Codable { + /// A relative or range threshold. And that is a logical "or", meaning any or both. public struct RelativeOrRange: Codable { - public struct Relative: Encodable { + public struct Relative { public let base: Int public let tolerancePercentage: Double - init(base: Int, tolerancePercentage: Double) { - precondition(base > 0, "base must be positive") - precondition(tolerancePercentage > 0, "tolerancePercentage must be positive") + public init(base: Int, tolerancePercentage: Double) { self.base = base self.tolerancePercentage = tolerancePercentage + + if !self.checkValid() { + print( + """ + Warning: Got invalid relative threshold values. base: \(self.base), tolerancePercentage: \(self.tolerancePercentage). + These must satisfy the following conditions: + - base must be non-negative + - tolerancePercentage must be finite + - tolerancePercentage must be non-negative + - tolerancePercentage must be less than or equal to 100 + """ + ) + } } /// Returns whether or not the value satisfies this relative range, as well as the /// percentage of the deviation of the value. public func contains(_ value: Int) -> (contains: Bool, deviation: Double) { - let deviation = Double(value - base) / Double(base) * 100 - return (abs(deviation) <= tolerancePercentage, deviation) + let diff = Double(value - base) + let allowedDiff = (Double(base) * tolerancePercentage / 100) + let allowedDiffWithPrecision = Statistics.roundToDecimalPlaces(allowedDiff, 6, .up) + let absDiffWithPrecision = Statistics.roundToDecimalPlaces(abs(diff), 6, .up) + let contains = absDiffWithPrecision <= allowedDiffWithPrecision + let deviation = (base == 0) ? 0 : diff / Double(base) * 100 + return (contains, deviation) + } + + func checkValid() -> Bool { + self.base >= 0 + && self.tolerancePercentage.isFinite + && self.tolerancePercentage >= 0 + && self.tolerancePercentage <= 100 } } - public struct Range: Encodable { + public struct Range { public let min: Int public let max: Int - init(min: Int, max: Int) { - precondition(min <= max, "min must be less than or equal to max") + public init(min: Int, max: Int) { self.min = min self.max = max + + if !self.checkValid() { + print( + """ + Warning: Got invalid range threshold values. min: \(self.min), max: \(self.max). + These must satisfy the following conditions: + - min must be less than or equal to max + """ + ) + } } public func contains(_ value: Int) -> Bool { return value >= min && value <= max } + + func checkValid() -> Bool { + self.min <= self.max + } } public let relative: Relative? public let range: Range? - init(relative: Relative?, range: Range?) { + public init(relative: Relative?, range: Range?) { self.relative = relative self.range = range - preconditionContainsAnyValue() + + if !self.checkValid() { + print( + """ + Warning: Got invalid RelativeOrRange threshold values. relative: \(String(describing: self.relative)), range: \(String(describing: self.range)). + At least one of the values must be non-nil. + """ + ) + } } - public func preconditionContainsAnyValue() { - precondition( - self.containsAnyValue, - "RelativeOrRange must contain either a relative or range, but contains neither" - ) + func checkValid() -> Bool { + self.containsAnyValue } var containsAnyValue: Bool { @@ -127,13 +169,18 @@ public enum BenchmarkThreshold: Codable { if let base, let tolerancePercentage { relative = Relative(base: base, tolerancePercentage: tolerancePercentage) - guard base > 0, tolerancePercentage > 0 else { + guard relative?.checkValid() != false else { throw DecodingError.dataCorrupted( .init( codingPath: decoder.codingPath, debugDescription: """ RelativeOrRange thresholds object contains an invalid relative values. - 'base' (\(base)) and 'tolerancePercentage' (\(tolerancePercentage)) must be positive. + base: \(base), tolerancePercentage: \(tolerancePercentage). + These must satisfy the following conditions: + - base must be non-negative + - tolerancePercentage must be finite + - tolerancePercentage must be non-negative + - tolerancePercentage must be less than or equal to 100 """ ) ) @@ -142,7 +189,7 @@ public enum BenchmarkThreshold: Codable { if let min, let max { range = Range(min: min, max: max) - guard min <= max else { + guard range?.checkValid() != false else { throw DecodingError.dataCorrupted( .init( codingPath: decoder.codingPath, @@ -177,8 +224,15 @@ public enum BenchmarkThreshold: Codable { } public func encode(to encoder: any Encoder) throws { - try self.relative?.encode(to: encoder) - try self.range?.encode(to: encoder) + var container = encoder.container(keyedBy: CodingKeys.self) + if let relative { + try container.encode(relative.base, forKey: .base) + try container.encode(relative.tolerancePercentage, forKey: .tolerancePercentage) + } + if let range { + try container.encode(range.min, forKey: .min) + try container.encode(range.max, forKey: .max) + } } } diff --git a/Sources/Benchmark/Statistics.swift b/Sources/Benchmark/Statistics.swift index 6af433aa..bd6c3198 100644 --- a/Sources/Benchmark/Statistics.swift +++ b/Sources/Benchmark/Statistics.swift @@ -181,10 +181,14 @@ public final class Statistics: Codable { } // Rounds decimals for display - public static func roundToDecimalPlaces(_ original: Double, _ decimals: Int = 2) -> Double { + public static func roundToDecimalPlaces( + _ original: Double, + _ decimals: Int = 2, + _ rule: FloatingPointRoundingRule = .toNearestOrEven + ) -> Double { let factor: Double = .pow(10.0, Double(decimals)) var original: Double = original * factor - original.round(.toNearestOrEven) + original.round(rule) return original / factor } } From 89835dec0d0a22c8713e9b86a6190886f7ee0b6f Mon Sep 17 00:00:00 2001 From: MahdiBM Date: Wed, 3 Dec 2025 19:34:32 +0330 Subject: [PATCH 5/5] Merge remote-tracking branch 'origin/mmbm-range-relative-thresholds-options-bak' into mmbm-range-relative-thresholds-options-bak --- .../Basic/BenchmarkRunner+Basic.swift | 2 +- Package.swift | 4 +- .../BenchmarkBoilerplateGenerator.swift | 2 +- .../BenchmarkCommandPlugin.swift | 10 ++--- .../BenchmarkTool+Baselines.swift | 40 +++++++++---------- .../BenchmarkTool+CreateBenchmark.swift | 16 ++++---- .../BenchmarkTool+Export+JMHFormatter.swift | 4 +- .../BenchmarkTool/BenchmarkTool+Machine.swift | 2 +- .../BenchmarkTool+Operations.swift | 14 +++---- ...chmarkTool+ReadP90AbsoluteThresholds.swift | 2 +- Plugins/BenchmarkTool/BenchmarkTool.swift | 12 +++--- .../BenchmarkTool/FilePath+Additions.swift | 2 +- .../Benchmark+ConvenienceInitializers.swift | 4 +- Sources/Benchmark/Benchmark.swift | 20 +++++----- Sources/Benchmark/BenchmarkExecutor.swift | 18 ++++----- Sources/Benchmark/BenchmarkInternals.swift | 8 ++-- Sources/Benchmark/BenchmarkMetric.swift | 16 ++++---- Sources/Benchmark/BenchmarkResult.swift | 24 +++++------ Sources/Benchmark/BenchmarkRunner.swift | 4 +- Sources/Benchmark/Blackhole.swift | 4 +- .../Benchmark/MallocStats/MallocStats.swift | 2 +- .../OperatingSystemStatsProducer+Linux.swift | 2 +- .../Benchmark/Progress/ProgressElements.swift | 2 +- Sources/Benchmark/Statistics.swift | 16 ++++---- .../BenchmarkTests/BenchmarkRunnerTests.swift | 2 +- 25 files changed, 115 insertions(+), 117 deletions(-) diff --git a/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift b/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift index 837c0cb1..3e7b4114 100644 --- a/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift +++ b/Benchmarks/Benchmarks/Basic/BenchmarkRunner+Basic.swift @@ -125,7 +125,7 @@ let benchmarks: @Sendable () -> Void = { } } - let parameterization = (0...5).map { 1 << $0 } // 1, 2, 4, ... + let parameterization = (0...5).map { 1 << $0 } // 1, 2, 4, ... parameterization.forEach { count in Benchmark("Parameterized", configuration: .init(tags: ["count": count.description])) { benchmark in diff --git a/Package.swift b/Package.swift index 72969970..e6790598 100644 --- a/Package.swift +++ b/Package.swift @@ -117,7 +117,7 @@ let package = Package( ) // Check if this is a SPI build, then we need to disable jemalloc for macOS -let macOSSPIBuild: Bool // Disables jemalloc for macOS SPI builds as the infrastructure doesn't have jemalloc there +let macOSSPIBuild: Bool // Disables jemalloc for macOS SPI builds as the infrastructure doesn't have jemalloc there #if canImport(Darwin) if let spiBuildEnvironment = ProcessInfo.processInfo.environment["SPI_BUILD"], spiBuildEnvironment == "1" { @@ -144,7 +144,7 @@ var dependencies: [PackageDescription.Target.Dependency] = [ "BenchmarkShared", ] -if macOSSPIBuild == false { // jemalloc always disable for macOSSPIBuild +if macOSSPIBuild == false { // jemalloc always disable for macOSSPIBuild if let disableJemalloc, disableJemalloc != "false", disableJemalloc != "0" { print("Jemalloc disabled through environment variable.") } else { diff --git a/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift b/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift index 03bcffd8..2da0884d 100644 --- a/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift +++ b/Plugins/BenchmarkBoilerplateGenerator/BenchmarkBoilerplateGenerator.swift @@ -20,7 +20,7 @@ struct Benchmark: AsyncParsableCommand { var output: String mutating func run() async throws { - let outputPath = FilePath(output) // package + let outputPath = FilePath(output) // package var boilerplate = """ import Benchmark diff --git a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift index b866b8cd..f84514f2 100644 --- a/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift +++ b/Plugins/BenchmarkCommandPlugin/BenchmarkCommandPlugin.swift @@ -37,7 +37,7 @@ import Glibc let specifiedTargets = try argumentExtractor.extractSpecifiedTargets(in: context.package, withOption: "target") let skipTargets = try argumentExtractor.extractSpecifiedTargets(in: context.package, withOption: "skip-target") let outputFormats = argumentExtractor.extractOption(named: "format") - let pathSpecified = argumentExtractor.extractOption(named: "path") // export path + let pathSpecified = argumentExtractor.extractOption(named: "path") // export path let quietRunning = argumentExtractor.extractFlag(named: "quiet") let noProgress = argumentExtractor.extractFlag(named: "no-progress") let checkAbsoluteThresholdsPath = argumentExtractor.extractOption(named: "check-absolute-path") @@ -143,16 +143,16 @@ import Glibc ) throw MyError.invalidArgument } - } catch { // We will throw if we can use the target name (it's unused!) + } catch { // We will throw if we can use the target name (it's unused!) } } let swiftSourceModuleTargets: [SwiftSourceModuleTarget] - var shouldBuildTargets = true // We don't rebuild the targets when we dont need to execute them, e.g. baseline read/compare + var shouldBuildTargets = true // We don't rebuild the targets when we dont need to execute them, e.g. baseline read/compare let packageBenchmarkIdentifier = "package-benchmark" let benchmarkToolName = "BenchmarkTool" - let benchmarkTool: PackagePlugin.Path // = try context.tool(named: benchmarkToolName) + let benchmarkTool: PackagePlugin.Path // = try context.tool(named: benchmarkToolName) var args: [String] = [ benchmarkToolName, @@ -473,7 +473,7 @@ import Glibc } let buildResult = try packageManager.build( - .product(target.name), // .all(includingTests: false), + .product(target.name), // .all(includingTests: false), parameters: .init(configuration: .release) ) diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift b/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift index c81aea90..edd3d615 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Baselines.swift @@ -33,8 +33,8 @@ struct BenchmarkMachine: Codable, Equatable { var hostname: String var processors: Int - var processorType: String // e.g. arm64e - var memory: Int // in GB + var processorType: String // e.g. arm64e + var memory: Int // in GB var kernelVersion: String public static func == (lhs: BenchmarkMachine, rhs: BenchmarkMachine) -> Bool { @@ -48,8 +48,8 @@ struct BenchmarkIdentifier: Codable, Hashable { self.name = name } - var target: String // The name of the executable benchmark target id - var name: String // The name of the benchmark + var target: String // The name of the executable benchmark target id + var name: String // The name of the benchmark public func hash(into hasher: inout Hasher) { hasher.combine(target) @@ -178,7 +178,7 @@ let baselinesDirectory: String = ".benchmarkBaselines" extension BenchmarkTool { func printAllBaselines() { var storagePath = FilePath(baselineStoragePath) - storagePath.append(baselinesDirectory) // package/.benchmarkBaselines + storagePath.append(baselinesDirectory) // package/.benchmarkBaselines for file in storagePath.directoryEntries { if file.ends(with: ".") == false, file.ends(with: "..") == false @@ -206,7 +206,7 @@ extension BenchmarkTool { var storagePath = FilePath(baselineStoragePath) let filemanager = FileManager.default - storagePath.append(baselinesDirectory) // package/.benchmarkBaselines + storagePath.append(baselinesDirectory) // package/.benchmarkBaselines for file in storagePath.directoryEntries { if file.ends(with: ".") == false, file.ends(with: "..") == false @@ -284,14 +284,14 @@ extension BenchmarkTool { │ └── ... └── ... */ - var outputPath = FilePath(baselineStoragePath) // package - var subPath = FilePath() // subpath rooted in package used for directory creation + var outputPath = FilePath(baselineStoragePath) // package + var subPath = FilePath() // subpath rooted in package used for directory creation - subPath.append(baselinesDirectory) // package/.benchmarkBaselines - subPath.append("\(target)") // package/.benchmarkBaselines/myTarget1 - subPath.append(baselineName) // package/.benchmarkBaselines/myTarget1/named1 + subPath.append(baselinesDirectory) // package/.benchmarkBaselines + subPath.append("\(target)") // package/.benchmarkBaselines/myTarget1 + subPath.append(baselineName) // package/.benchmarkBaselines/myTarget1/named1 - outputPath.createSubPath(subPath) // Create destination subpath if needed + outputPath.createSubPath(subPath) // Create destination subpath if needed outputPath.append(subPath.components) @@ -348,13 +348,13 @@ extension BenchmarkTool { baselineIdentifier: String? = nil ) throws -> BenchmarkBaseline? { var path = FilePath(baselineStoragePath) - path.append(baselinesDirectory) // package/.benchmarkBaselines - path.append(FilePath.Component(target)!) // package/.benchmarkBaselines/myTarget1 + path.append(baselinesDirectory) // package/.benchmarkBaselines + path.append(FilePath.Component(target)!) // package/.benchmarkBaselines/myTarget1 if let baselineIdentifier { - path.append(baselineIdentifier) // package/.benchmarkBaselines/myTarget1/named1 + path.append(baselineIdentifier) // package/.benchmarkBaselines/myTarget1/named1 } else { - path.append("default") // // package/.benchmarkBaselines/myTarget1/default + path.append("default") // // package/.benchmarkBaselines/myTarget1/default } if let hostIdentifier { @@ -376,7 +376,7 @@ extension BenchmarkTool { let bufferSize = 16 * 1_024 * 1_024 var done = false - while done == false { // readBytes.count < bufferLength { + while done == false { // readBytes.count < bufferLength { let nextBytes = try [UInt8](unsafeUninitializedCapacity: bufferSize) { buf, count in count = try fd.read(into: UnsafeMutableRawBufferPointer(buf)) if count == 0 { @@ -396,7 +396,7 @@ extension BenchmarkTool { print("Failed to close fd for \(path) after reading.") } } catch { - if errno != ENOENT { // file not found is ok, e.g. when no baselines exist + if errno != ENOENT { // file not found is ok, e.g. when no baselines exist print("Failed to open file \(path), errno = [\(errno)]") } } @@ -522,11 +522,11 @@ extension BenchmarkBaseline: Equatable { for (lhsBenchmarkIdentifier, lhsBenchmarkResults) in lhs.results { for lhsBenchmarkResult in lhsBenchmarkResults { - guard let rhsResults = rhs.results.first(where: { $0.key == lhsBenchmarkIdentifier }) else { // We couldn't find a result for one of the tests + guard let rhsResults = rhs.results.first(where: { $0.key == lhsBenchmarkIdentifier }) else { // We couldn't find a result for one of the tests return false } guard let rhsBenchmarkResult = rhsResults.value.first(where: { $0.metric == lhsBenchmarkResult.metric }) - else { // We couldn't find the specific metric + else { // We couldn't find the specific metric return false } if lhsBenchmarkResult != rhsBenchmarkResult { diff --git a/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift b/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift index e48f8f61..86c7c574 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+CreateBenchmark.swift @@ -44,9 +44,9 @@ extension BenchmarkTool { ] """ - var outputPath = FilePath(baselineStoragePath) // package - var subPath = FilePath() // subpath rooted in package used for directory creation - subPath.append("Package.swift") // package/Benchmarks/targetName + var outputPath = FilePath(baselineStoragePath) // package + var subPath = FilePath() // subpath rooted in package used for directory creation + subPath.append("Package.swift") // package/Benchmarks/targetName outputPath.append(subPath.components) print("Adding new executable target \(targetName) to \(outputPath.description)") @@ -110,13 +110,13 @@ extension BenchmarkTool { """ - var outputPath = FilePath(baselineStoragePath) // package - var subPath = FilePath() // subpath rooted in package used for directory creation + var outputPath = FilePath(baselineStoragePath) // package + var subPath = FilePath() // subpath rooted in package used for directory creation - subPath.append(benchmarksDirectory) // package/Benchmarks - subPath.append("\(targetName)") // package/Benchmarks/targetName + subPath.append(benchmarksDirectory) // package/Benchmarks + subPath.append("\(targetName)") // package/Benchmarks/targetName - outputPath.createSubPath(subPath) // Create destination subpath if needed + outputPath.createSubPath(subPath) // Create destination subpath if needed outputPath.append(subPath.components) outputPath.append("\(targetName).swift") diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift b/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift index cd15b664..8c5ef5d3 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Export+JMHFormatter.swift @@ -56,7 +56,7 @@ extension JMHPrimaryMetric { if result.metric.countable { scoreUnit = result.metric == .throughput ? "# / s" : "#" } else { - scoreUnit = "μs" // result.timeUnits.description + scoreUnit = "μs" // result.timeUnits.description } rawData = [recordedValues] } @@ -66,7 +66,7 @@ extension BenchmarkTool { func convertToJMH(_ baseline: BenchmarkBaseline) throws -> String { var resultString = "" var jmhElements: [JMHElement] = [] - var secondaryMetrics: [String: JMHPrimaryMetric] = [:] // could move to OrderedDictionary for consistent output + var secondaryMetrics: [String: JMHPrimaryMetric] = [:] // could move to OrderedDictionary for consistent output baseline.targets.forEach { benchmarkTarget in diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift b/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift index 341add71..08934f49 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Machine.swift @@ -23,7 +23,7 @@ import Glibc extension BenchmarkTool { func benchmarkMachine() -> BenchmarkMachine { let processors = sysconf(Int32(_SC_NPROCESSORS_ONLN)) - let memory = sysconf(Int32(_SC_PHYS_PAGES)) / 1_024 * sysconf(Int32(_SC_PAGESIZE)) / (1_024 * 1_024) // avoid overflow + let memory = sysconf(Int32(_SC_PHYS_PAGES)) / 1_024 * sysconf(Int32(_SC_PAGESIZE)) / (1_024 * 1_024) // avoid overflow var uuname = utsname() _ = uname(&uuname) diff --git a/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift b/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift index 86b243e6..b3454b83 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+Operations.swift @@ -29,7 +29,7 @@ extension BenchmarkTool { let benchmarkReply = try read() switch benchmarkReply { - case let .list(benchmark): + case .list(let benchmark): benchmark.executablePath = benchmarkPath benchmark.target = FilePath(benchmarkPath).lastComponent!.description if metrics.isEmpty == false { @@ -38,7 +38,7 @@ extension BenchmarkTool { benchmarks.append(benchmark) case .end: break outerloop - case let .error(description): + case .error(let description): failBenchmark(description) break outerloop default: @@ -55,12 +55,12 @@ extension BenchmarkTool { let benchmarkReply = try read() switch benchmarkReply { - case let .result(benchmark: benchmark, results: results): + case .result(benchmark: let benchmark, results: let results): let filteredResults = results.filter { benchmark.configuration.metrics.contains($0.metric) } benchmarkResults[BenchmarkIdentifier(target: target, name: benchmark.name)] = filteredResults case .end: break outerloop - case let .error(description): + case .error(let description): failBenchmark(description, exitCode: .benchmarkJobFailed, "\(target)/\(benchmark.name)") benchmarkResults[BenchmarkIdentifier(target: target, name: benchmark.name)] = [] @@ -134,7 +134,7 @@ extension BenchmarkTool { return } - if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare + if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare currentBaseline.results.keys.forEach { baselineKey in if let benchmark: Benchmark = .init(baselineKey.name, closure: { _ in }) { benchmark.target = baselineKey.target @@ -287,7 +287,7 @@ extension BenchmarkTool { return } - if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare + if benchmarks.isEmpty { // if we read from baseline and didn't run them, we put in some fake entries for the compare currentBaseline.results.keys.forEach { baselineKey in if let benchmark: Benchmark = .init(baselineKey.name, closure: { _ in }) { benchmark.target = baselineKey.target @@ -307,7 +307,7 @@ extension BenchmarkTool { var p90Thresholds: [BenchmarkIdentifier: [BenchmarkMetric: BenchmarkThreshold]] = [:] - if let benchmarkPath = checkAbsolutePath { // load statically defined thresholds for .p90 + if let benchmarkPath = checkAbsolutePath { // load statically defined thresholds for .p90 benchmarks.forEach { benchmark in if let thresholds = BenchmarkTool.makeBenchmarkThresholds( path: benchmarkPath, diff --git a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift index f40a4d46..0c304d73 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool+ReadP90AbsoluteThresholds.swift @@ -96,7 +96,7 @@ extension BenchmarkTool { print("Failed to close fd for \(path) after reading.") } } catch { - if errno != ENOENT { // file not found is ok, e.g. no thresholds found, then silently return nil + if errno != ENOENT { // file not found is ok, e.g. no thresholds found, then silently return nil print("Failed to open file \(path), errno = [\(errno)] \(Errno(rawValue: errno).description)") } } diff --git a/Plugins/BenchmarkTool/BenchmarkTool.swift b/Plugins/BenchmarkTool/BenchmarkTool.swift index 974a68f3..b35ee1f9 100644 --- a/Plugins/BenchmarkTool/BenchmarkTool.swift +++ b/Plugins/BenchmarkTool/BenchmarkTool.swift @@ -28,7 +28,7 @@ enum BenchmarkOperation: String, ExpressibleByArgument { case thresholds case list case run - case query // query all benchmarks from target, used internally in tool + case query // query all benchmarks from target, used internally in tool case `init` } @@ -136,7 +136,7 @@ struct BenchmarkTool: AsyncParsableCommand { var outputFD: CInt = 0 var benchmarks: [Benchmark] = [] - var benchmarkBaselines: [BenchmarkBaseline] = [] // The baselines read from disk, merged + current run if needed + var benchmarkBaselines: [BenchmarkBaseline] = [] // The baselines read from disk, merged + current run if needed var comparisonBaseline: BenchmarkBaseline? var checkBaseline: BenchmarkBaseline? @@ -201,9 +201,9 @@ struct BenchmarkTool: AsyncParsableCommand { mutating func readBaselines() throws { func readBaseline(_ baselineName: String) throws -> BenchmarkBaseline? { // read all specified baselines - var readBaselines: [BenchmarkBaseline] = [] // The baselines read from disk + var readBaselines: [BenchmarkBaseline] = [] // The baselines read from disk - try targets.forEach { target in // read from all the targets (baselines are stored separately) + try targets.forEach { target in // read from all the targets (baselines are stored separately) let currentBaseline = try read(target: target, baselineIdentifier: baselineName) if let currentBaseline { @@ -223,7 +223,7 @@ struct BenchmarkTool: AsyncParsableCommand { return nil } - try baseline.forEach { baselineName in // for all specified baselines at command line + try baseline.forEach { baselineName in // for all specified baselines at command line if let baseline = try readBaseline(baselineName) { benchmarkBaselines.append(baseline) } else { @@ -413,7 +413,7 @@ struct BenchmarkTool: AsyncParsableCommand { case .`init`: fatalError("Should never come here") case .query: - try queryBenchmarks(benchmarkPath) // Get all available benchmarks first + try queryBenchmarks(benchmarkPath) // Get all available benchmarks first case .list: try listBenchmarks() case .baseline, .thresholds, .run: diff --git a/Plugins/BenchmarkTool/FilePath+Additions.swift b/Plugins/BenchmarkTool/FilePath+Additions.swift index 228bc7a6..7659e7cc 100644 --- a/Plugins/BenchmarkTool/FilePath+Additions.swift +++ b/Plugins/BenchmarkTool/FilePath+Additions.swift @@ -38,7 +38,7 @@ public extension FilePath { } catch { print("failed close directory") } } catch { switch errno { - case ENOENT: // doesn't exist, let's create it + case ENOENT: // doesn't exist, let's create it if mkdir(creationPath.string, S_IRWXU | S_IRWXG | S_IRWXO) == -1 { if errno == EPERM { print("Lacking permissions to write to \(creationPath)") diff --git a/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift b/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift index 61212f21..e40eb651 100644 --- a/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift +++ b/Sources/Benchmark/Benchmark+ConvenienceInitializers.swift @@ -17,7 +17,7 @@ public extension Benchmark { teardown: BenchmarkTeardownHook? = nil ) { self.init(name, configuration: configuration) { benchmark in - let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast + let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast closure(benchmark, setupResult) } teardown: { try await teardown?() @@ -46,7 +46,7 @@ public extension Benchmark { teardown: BenchmarkTeardownHook? = nil ) { self.init(name, configuration: configuration) { benchmark in - let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast + let setupResult = benchmark.setupState! as! SetupResult // swiftlint:disable:this force_cast await closure(benchmark, setupResult) } teardown: { try await teardown?() diff --git a/Sources/Benchmark/Benchmark.swift b/Sources/Benchmark/Benchmark.swift index c0bc7f44..44e811f2 100644 --- a/Sources/Benchmark/Benchmark.swift +++ b/Sources/Benchmark/Benchmark.swift @@ -14,7 +14,7 @@ import Foundation // swiftlint:disable file_length identifier_name /// Defines a benchmark -public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type_body_length +public final class Benchmark: Codable, Hashable { // swiftlint:disable:this type_body_length @_documentation(visibility: internal) public typealias BenchmarkClosure = (_ benchmark: Benchmark) -> Void @_documentation(visibility: internal) @@ -36,11 +36,11 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this typ @_documentation(visibility: internal) @ThreadSafeProperty(wrappedValue: nil, lock: setupTeardownLock) - public static var _startupHook: BenchmarkSetupHook? // Should be removed when going to 2.0, just kept for API compatiblity + public static var _startupHook: BenchmarkSetupHook? // Should be removed when going to 2.0, just kept for API compatiblity @_documentation(visibility: internal) @ThreadSafeProperty(wrappedValue: nil, lock: setupTeardownLock) - public static var _shutdownHook: BenchmarkTeardownHook? // Should be removed when going to 2.0, just kept for API compatiblity + public static var _shutdownHook: BenchmarkTeardownHook? // Should be removed when going to 2.0, just kept for API compatiblity @_documentation(visibility: internal) @ThreadSafeProperty(wrappedValue: nil, lock: setupTeardownLock) @@ -111,7 +111,7 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this typ public static var checkAbsoluteThresholds = false @_documentation(visibility: internal) - public static var benchmarks: [Benchmark] = [] // Bookkeeping of all registered benchmarks + public static var benchmarks: [Benchmark] = [] // Bookkeeping of all registered benchmarks /// The name of the benchmark without any of the tags appended public var baseName: String @@ -150,9 +150,9 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this typ @_documentation(visibility: internal) public var executablePath: String? /// closure: The actual benchmark closure that will be measured - var closure: BenchmarkClosure? // The actual benchmark to run + var closure: BenchmarkClosure? // The actual benchmark to run /// asyncClosure: The actual benchmark (async) closure that will be measured - var asyncClosure: BenchmarkAsyncClosure? // The actual benchmark to run + var asyncClosure: BenchmarkAsyncClosure? // The actual benchmark to run // setup/teardown hooks for the instance var setup: BenchmarkSetupHook? var teardown: BenchmarkTeardownHook? @@ -205,8 +205,8 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this typ } #endif - static var testSkipBenchmarkRegistrations = false // true in test to avoid bench registration fail - var measurementCompleted = false // Keep track so we skip multiple 'end of measurement' + static var testSkipBenchmarkRegistrations = false // true in test to avoid bench registration fail + var measurementCompleted = false // Keep track so we skip multiple 'end of measurement' enum CodingKeys: String, CodingKey { case baseName = "name" @@ -406,7 +406,7 @@ public final class Benchmark: Codable, Hashable { // swiftlint:disable:this typ } private func _stopMeasurement(_ explicitStartStop: Bool) { - guard measurementCompleted == false else { // This is to skip the implicit stop if we did an explicit before + guard measurementCompleted == false else { // This is to skip the implicit stop if we did an explicit before return } @@ -560,7 +560,7 @@ public extension Benchmark { /// } /// } /// ``` - @_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf + @_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf static func blackHole(_: some Any) {} } diff --git a/Sources/Benchmark/BenchmarkExecutor.swift b/Sources/Benchmark/BenchmarkExecutor.swift index 6c640ff6..99e07086 100644 --- a/Sources/Benchmark/BenchmarkExecutor.swift +++ b/Sources/Benchmark/BenchmarkExecutor.swift @@ -14,7 +14,7 @@ import OSLog // swiftlint:disable file_length -struct BenchmarkExecutor { // swiftlint:disable:this type_body_length +struct BenchmarkExecutor { // swiftlint:disable:this type_body_length init(quiet: Bool = false) { self.quiet = quiet } @@ -107,7 +107,7 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length let initialStartTime = BenchmarkClock.now // 'Warmup' to remove initial mallocs from stats in p100 - _ = MallocStatsProducer.makeMallocStats() // baselineMallocStats + _ = MallocStatsProducer.makeMallocStats() // baselineMallocStats // Calculate typical sys call check overhead and deduct that to get 'clean' stats for the actual benchmark var operatingSystemStatsOverhead = OperatingSystemStats() @@ -129,7 +129,7 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length for _ in 0.. .zero { // macOS sometimes gives us identical timestamps so let's skip those. + if runningTime > .zero { // macOS sometimes gives us identical timestamps so let's skip those. let nanoSeconds = runningTime.nanoseconds() statistics[BenchmarkMetric.wallClock.index].add(Int(nanoSeconds)) @@ -228,10 +228,10 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length let objectAllocDelta = stopARCStats.objectAllocCount - startARCStats.objectAllocCount statistics[BenchmarkMetric.objectAllocCount.index].add(Int(objectAllocDelta)) - let retainDelta = stopARCStats.retainCount - startARCStats.retainCount - 1 // due to some ARC traffic in the path + let retainDelta = stopARCStats.retainCount - startARCStats.retainCount - 1 // due to some ARC traffic in the path statistics[BenchmarkMetric.retainCount.index].add(Int(retainDelta)) - let releaseDelta = stopARCStats.releaseCount - startARCStats.releaseCount - 1 // due to some ARC traffic in the path + let releaseDelta = stopARCStats.releaseCount - startARCStats.releaseCount - 1 // due to some ARC traffic in the path statistics[BenchmarkMetric.releaseCount.index].add(Int(releaseDelta)) statistics[BenchmarkMetric.retainReleaseDelta.index] @@ -399,7 +399,7 @@ struct BenchmarkExecutor { // swiftlint:disable:this type_body_length iterations += 1 - if iterations < 1_000 || iterations.isMultiple(of: 500) { // only update for low iteration count benchmarks, else 1/500 + if iterations < 1_000 || iterations.isMultiple(of: 500) { // only update for low iteration count benchmarks, else 1/500 if var progressBar { let iterationsPercentage = 100.0 * Double(iterations) / Double(benchmark.configuration.maxIterations) diff --git a/Sources/Benchmark/BenchmarkInternals.swift b/Sources/Benchmark/BenchmarkInternals.swift index e942c577..1c624116 100644 --- a/Sources/Benchmark/BenchmarkInternals.swift +++ b/Sources/Benchmark/BenchmarkInternals.swift @@ -17,7 +17,7 @@ public enum BenchmarkCommandRequest: Codable { case list case run(benchmark: Benchmark) - case end // exit the benchmark + case end // exit the benchmark } // Replies from benchmark under measure to benchmark runner @@ -25,10 +25,10 @@ public enum BenchmarkCommandRequest: Codable { public enum BenchmarkCommandReply: Codable { case list(benchmark: Benchmark) case ready - case result(benchmark: Benchmark, results: [BenchmarkResult]) // receives results from built-in metric collectors + case result(benchmark: Benchmark, results: [BenchmarkResult]) // receives results from built-in metric collectors case run - case end // end of query for list/result - case error(_ description: String) // error while performing operation (e.g. 'run') + case end // end of query for list/result + case error(_ description: String) // error while performing operation (e.g. 'run') } // swiftlint:enable all diff --git a/Sources/Benchmark/BenchmarkMetric.swift b/Sources/Benchmark/BenchmarkMetric.swift index 3a0edf9c..b1032cc3 100644 --- a/Sources/Benchmark/BenchmarkMetric.swift +++ b/Sources/Benchmark/BenchmarkMetric.swift @@ -96,7 +96,7 @@ public extension BenchmarkMetric { public extension BenchmarkMetric { /// A constant that states whether larger or smaller measurements, relative to a set baseline, indicate better performance. - enum Polarity: Codable, Sendable { // same naming as XCTest uses, polarity is known for all metrics except custom + enum Polarity: Codable, Sendable { // same naming as XCTest uses, polarity is known for all metrics except custom /// A performance measurement where a larger value, relative to a set baseline, indicates better performance. case prefersLarger /// A performance measurement where a smaller value, relative to a set baseline, indicates better performance. @@ -132,7 +132,7 @@ public extension BenchmarkMetric { return true case .objectAllocCount, .retainCount, .releaseCount, .retainReleaseDelta: return true - case let .custom(_, _, useScaleFactor): + case .custom(_, _, let useScaleFactor): return useScaleFactor default: return false @@ -144,7 +144,7 @@ public extension BenchmarkMetric { switch self { case .throughput: return .prefersLarger - case let .custom(_, polarity, _): + case .custom(_, let polarity, _): return polarity default: return .prefersSmaller @@ -213,7 +213,7 @@ public extension BenchmarkMetric { return "Δ" case .deltaPercentage: return "Δ %" - case let .custom(name, _, _): + case .custom(let name, _, _): return name } } @@ -279,7 +279,7 @@ public extension BenchmarkMetric { case .instructions: return 28 default: - return 0 // custom payloads must be stored in dictionary + return 0 // custom payloads must be stored in dictionary } } @@ -288,7 +288,7 @@ public extension BenchmarkMetric { // Used by the Benchmark Executor for efficient indexing into results @_documentation(visibility: internal) - func metricFor(index: Int) -> BenchmarkMetric { // swiftlint:disable:this cyclomatic_complexity function_body_length + func metricFor(index: Int) -> BenchmarkMetric { // swiftlint:disable:this cyclomatic_complexity function_body_length switch index { case 1: return .cpuUser @@ -355,7 +355,7 @@ public extension BenchmarkMetric { @_documentation(visibility: internal) public extension BenchmarkMetric { - var rawDescription: String { // As we can't have raw values due to custom support, we do this... + var rawDescription: String { // As we can't have raw values due to custom support, we do this... switch self { case .cpuUser: return "cpuUser" @@ -417,7 +417,7 @@ public extension BenchmarkMetric { return "Δ" case .deltaPercentage: return "Δ %" - case let .custom(name, _, _): + case .custom(let name, _, _): return name } } diff --git a/Sources/Benchmark/BenchmarkResult.swift b/Sources/Benchmark/BenchmarkResult.swift index 5b0fd5cd..394dfe51 100644 --- a/Sources/Benchmark/BenchmarkResult.swift +++ b/Sources/Benchmark/BenchmarkResult.swift @@ -31,7 +31,7 @@ public enum BenchmarkTimeUnits: String, Codable, CustomStringConvertible, CaseIt case seconds case kiloseconds case megaseconds - case automatic // will pick time unit above automatically + case automatic // will pick time unit above automatically public var factor: Int { switch self { case .nanoseconds: @@ -43,7 +43,7 @@ public enum BenchmarkTimeUnits: String, Codable, CustomStringConvertible, CaseIt case .seconds: return 1 case .kiloseconds: - return 2 // Yeah, not right but we need to refactor to get rid of this, works for now + return 2 // Yeah, not right but we need to refactor to get rid of this, works for now case .megaseconds: return 3 case .automatic: @@ -98,7 +98,7 @@ public enum BenchmarkUnits: Int, Codable, CustomStringConvertible, CaseIterable case giga = 1_000_000_000 case tera = 1_000_000_000_000 case peta = 1_000_000_000_000_000 - case automatic // will pick unit above automatically + case automatic // will pick unit above automatically public var description: String { switch self { @@ -169,17 +169,17 @@ public extension BenchmarkTimeUnits { /// Use a scaling factor when running your short benchmarks to provide greater numerical stability to the results. public enum BenchmarkScalingFactor: Int, Codable { /// No scaling factor, the raw count of iterations. - case one = 1 // e.g. nanoseconds, or count + case one = 1 // e.g. nanoseconds, or count /// Scaling factor of 1e03. - case kilo = 1_000 // microseconds + case kilo = 1_000 // microseconds /// Scaling factor of 1e06. - case mega = 1_000_000 // milliseconds + case mega = 1_000_000 // milliseconds /// Scaling factor of 1e09. - case giga = 1_000_000_000 // seconds + case giga = 1_000_000_000 // seconds /// Scaling factor of 1e12. - case tera = 1_000_000_000_000 // 1K seconds + case tera = 1_000_000_000_000 // 1K seconds /// Scaling factor of 1e15. - case peta = 1_000_000_000_000_000 // 1M + case peta = 1_000_000_000_000_000 // 1M public var description: String { switch self { @@ -279,7 +279,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { return .microseconds case .giga: return .nanoseconds - case .tera, .peta: // shouldn't be possible as tera is only used internally to present scaled up throughput + case .tera, .peta: // shouldn't be possible as tera is only used internally to present scaled up throughput break } default: @@ -312,7 +312,7 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { return y * x } else if n.isMultiple(of: 2) { return expBySq(y, x * x, n / 2) - } else { // n is odd + } else { // n is odd return expBySq(y * x, x * x, (n - 1) / 2) } } @@ -669,7 +669,6 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { .absolute(rhsPercentiles[percentile]), Self.Percentile(rawValue: percentile)!, thresholds, - lhs.statistics.units(), &thresholdResults, name, target @@ -695,7 +694,6 @@ public struct BenchmarkResult: Codable, Comparable, Equatable { p90Threshold, .p90, thresholds, - statistics.units(), &thresholdResults, name, target diff --git a/Sources/Benchmark/BenchmarkRunner.swift b/Sources/Benchmark/BenchmarkRunner.swift index 4946da62..4c109382 100644 --- a/Sources/Benchmark/BenchmarkRunner.swift +++ b/Sources/Benchmark/BenchmarkRunner.swift @@ -119,7 +119,7 @@ public struct BenchmarkRunner: AsyncParsableCommand, BenchmarkRunnerReadWrite { let suppressor = OutputSuppressor() while true { - if debug { // in debug mode we run all benchmarks matching filter/skip specified + if debug { // in debug mode we run all benchmarks matching filter/skip specified var benchmark: Benchmark? benchmarkCommand = .list @@ -147,7 +147,7 @@ public struct BenchmarkRunner: AsyncParsableCommand, BenchmarkRunnerReadWrite { } try channel.write(.end) - case let .run(benchmarkToRun): + case .run(let benchmarkToRun): benchmark = Benchmark.benchmarks.first { $0.name == benchmarkToRun.name } if let benchmark { diff --git a/Sources/Benchmark/Blackhole.swift b/Sources/Benchmark/Blackhole.swift index 37642626..bd6bae62 100644 --- a/Sources/Benchmark/Blackhole.swift +++ b/Sources/Benchmark/Blackhole.swift @@ -29,10 +29,10 @@ /// } /// } /// ``` -@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf +@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf public func blackHole(_: some Any) {} -@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf +@_optimize(none) // Used after tip here: https://forums.swift.org/t/compiler-swallows-blackhole/64305/10 - see also https://github.com/apple/swift/commit/1fceeab71e79dc96f1b6f560bf745b016d7fcdcf public func identity(_ value: T) -> T { value } diff --git a/Sources/Benchmark/MallocStats/MallocStats.swift b/Sources/Benchmark/MallocStats/MallocStats.swift index 14e1c10e..2056d413 100644 --- a/Sources/Benchmark/MallocStats/MallocStats.swift +++ b/Sources/Benchmark/MallocStats/MallocStats.swift @@ -23,5 +23,5 @@ struct MallocStats { /// , and unused dirty pages. This is a maximum rather than precise because pages may /// not actually be physically resident if they correspond to demand-zeroed virtual memory /// that has not yet been touched. This is a multiple of the page size. - var allocatedResidentMemory: Int = 0 // in bytes + var allocatedResidentMemory: Int = 0 // in bytes } diff --git a/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift b/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift index d464d3ba..ee35d299 100644 --- a/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift +++ b/Sources/Benchmark/OperatingSystemStats/OperatingSystemStatsProducer+Linux.swift @@ -205,7 +205,7 @@ final class OperatingSystemStatsProducer { self.lock.unlock() - if firstEventSampled == false { // allow calling thread to continue when we have captured a sample + if firstEventSampled == false { // allow calling thread to continue when we have captured a sample firstEventSampled = true sampleSemaphore.signal() } diff --git a/Sources/Benchmark/Progress/ProgressElements.swift b/Sources/Benchmark/Progress/ProgressElements.swift index 95920beb..26c4efa5 100644 --- a/Sources/Benchmark/Progress/ProgressElements.swift +++ b/Sources/Benchmark/Progress/ProgressElements.swift @@ -83,7 +83,7 @@ public struct ProgressPercent: ProgressElementType { while padded.count < 4 { padded = " " + padded } - return padded // "\(percentDone.format(decimalPlaces))%" + return padded // "\(percentDone.format(decimalPlaces))%" } } diff --git a/Sources/Benchmark/Statistics.swift b/Sources/Benchmark/Statistics.swift index bd6c3198..95639f8e 100644 --- a/Sources/Benchmark/Statistics.swift +++ b/Sources/Benchmark/Statistics.swift @@ -15,18 +15,18 @@ import Numerics // A type that provides distribution / percentile calculations of latency measurements @_documentation(visibility: internal) public final class Statistics: Codable { - public static let defaultMaximumMeasurement = 1_000_000_000 // 1 second in nanoseconds + public static let defaultMaximumMeasurement = 1_000_000_000 // 1 second in nanoseconds public static let defaultPercentilesToCalculate = [0.0, 25.0, 50.0, 75.0, 90.0, 99.0, 100.0] public static let defaultPercentilesToCalculateP90Index = 4 public enum Units: Int, Codable, CaseIterable { - case count = 1 // e.g. nanoseconds - case kilo = 1_000 // microseconds - case mega = 1_000_000 // milliseconds - case giga = 1_000_000_000 // seconds - case tera = 1_000_000_000_000 // 1K seconds - case peta = 1_000_000_000_000_000 // 1M seconds - case automatic = 0 // will pick time unit above automatically + case count = 1 // e.g. nanoseconds + case kilo = 1_000 // microseconds + case mega = 1_000_000 // milliseconds + case giga = 1_000_000_000 // seconds + case tera = 1_000_000_000_000 // 1K seconds + case peta = 1_000_000_000_000_000 // 1M seconds + case automatic = 0 // will pick time unit above automatically public var description: String { switch self { diff --git a/Tests/BenchmarkTests/BenchmarkRunnerTests.swift b/Tests/BenchmarkTests/BenchmarkRunnerTests.swift index a7724d65..31f17580 100644 --- a/Tests/BenchmarkTests/BenchmarkRunnerTests.swift +++ b/Tests/BenchmarkTests/BenchmarkRunnerTests.swift @@ -56,7 +56,7 @@ final class BenchmarkRunnerTests: XCTestCase, BenchmarkRunnerReadWrite { runner.quiet = false runner.timeUnits = .nanoseconds try await runner.run() - XCTAssertEqual(writeCount, 6) // 3 tests results + 3 end markers + XCTAssertEqual(writeCount, 6) // 3 tests results + 3 end markers } }