Repository: google/swift-benchmark
Pull request #54 (2020-06-25 15:42:49)

Fix missing time unit conversions in JSON and CSV output
This change makes JSON and CSV report results in time adjusted time units (i.e. adjust ns to ms based on `--time-unit ms`).

Previously, only Console reporter would adjust time units. Now the functionality is factored out and is used by all reporters uniformly. 


Patch:
diff --git a/Sources/Benchmark/BenchmarkArguments.swift b/Sources/Benchmark/BenchmarkArguments.swift
index 3fc8c86..b2afee5 100644
--- a/Sources/Benchmark/BenchmarkArguments.swift
+++ b/Sources/Benchmark/BenchmarkArguments.swift
@@ -138,3 +138,8 @@ public struct BenchmarkArguments: ParsableArguments {
         return "Value provided via \(flag) must be a non-negative \(type)."
     }
 }
+
+public func parseArguments() -> [BenchmarkSetting] {
+    let command = BenchmarkCommand.parseOrExit()
+    return command.arguments.settings
+}
diff --git a/Sources/Benchmark/BenchmarkColumn.swift b/Sources/Benchmark/BenchmarkColumn.swift
index 11a68a3..8051670 100644
--- a/Sources/Benchmark/BenchmarkColumn.swift
+++ b/Sources/Benchmark/BenchmarkColumn.swift
@@ -24,12 +24,21 @@ public struct BenchmarkColumn: Hashable {
     /// Function to compute a value for each cell based on results.
     public let value: (BenchmarkResult) -> Double
 
+    /// Unit of the column value.
+    public let unit: Unit
+
     /// Visual alignment to either left or right side of the column.
     public let alignment: Alignment
 
     /// Formatter function for pretty human-readable console output.
     public let formatter: Formatter
 
+    public enum Unit: Hashable {
+        case time
+        case inverseTime
+        case none
+    }
+
     public enum Alignment: Hashable {
         case left
         case right
@@ -38,13 +47,26 @@ public struct BenchmarkColumn: Hashable {
     public init(
         name: String,
         value: @escaping (BenchmarkResult) -> Double,
-        alignment: Alignment,
-        formatter: @escaping Formatter = BenchmarkFormatter.number
+        unit: Unit = .none,
+        alignment: Alignment = .right,
+        formatter optionalFormatter: Formatter? = nil 
     ) {
         self.name = name
         self.value = value
+        self.unit = unit
         self.alignment = alignment
-        self.formatter = formatter
+        if let formatter = optionalFormatter {
+            self.formatter = formatter
+        } else {
+            switch unit {
+            case .time:
+                self.formatter = BenchmarkFormatter.time
+            case .inverseTime:
+                self.formatter = BenchmarkFormatter.inverseTime
+            case .none:
+                self.formatter = BenchmarkFormatter.number
+            }
+        }
     }
 
     /// Registry that represents a mapping from known column
@@ -62,8 +84,7 @@ public struct BenchmarkColumn: Hashable {
         result["time"] = BenchmarkColumn(
             name: "time",
             value: { $0.measurements.median },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["std"] = BenchmarkColumn(
             name: "std",
             value: { $0.measurements.std / $0.measurements.median * 100 },
@@ -71,21 +92,17 @@ public struct BenchmarkColumn: Hashable {
             formatter: BenchmarkFormatter.stdPercentage)
         result["iterations"] = BenchmarkColumn(
             name: "iterations",
-            value: { Double($0.measurements.count) },
-            alignment: .right,
-            formatter: BenchmarkFormatter.number)
+            value: { Double($0.measurements.count) })
         result["warmup"] = BenchmarkColumn(
             name: "warmup",
             value: { $0.warmupMeasurements.sum },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
 
         // Opt-in alternative columns.
         result["median"] = BenchmarkColumn(
             name: "median",
             value: { $0.measurements.median },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["min"] = BenchmarkColumn(
             name: "min",
             value: { result in
@@ -95,8 +112,7 @@ public struct BenchmarkColumn: Hashable {
                     return (0)
                 }
             },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["max"] = BenchmarkColumn(
             name: "max",
             value: { result in
@@ -106,23 +122,19 @@ public struct BenchmarkColumn: Hashable {
                     return (0)
                 }
             },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["total"] = BenchmarkColumn(
             name: "total",
             value: { ($0.measurements.sum) },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["avg"] = BenchmarkColumn(
             name: "avg",
             value: { ($0.measurements.average) },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["average"] = BenchmarkColumn(
             name: "avg",
             value: { ($0.measurements.average) },
-            alignment: .right,
-            formatter: BenchmarkFormatter.time)
+            unit: .time)
         result["std_abs"] = BenchmarkColumn(
             name: "std_abs",
             value: { ($0.measurements.std) },
@@ -139,8 +151,7 @@ public struct BenchmarkColumn: Hashable {
             result[name] = BenchmarkColumn(
                 name: name,
                 value: { ($0.measurements.percentile(v)) },
-                alignment: .left,
-                formatter: BenchmarkFormatter.time)
+                unit: .time)
         }
 
         return result
@@ -234,10 +245,29 @@ public struct BenchmarkColumn: Hashable {
                     }
                 } else {
                     let value = column.value(result)
+                    let adjustedValue: Double
+                    switch column.unit {
+                    case .time:
+                        switch result.settings.timeUnit {
+                        case .ns: adjustedValue = value
+                        case .us: adjustedValue = value / 1000.0
+                        case .ms: adjustedValue = value / 1000_000.0
+                        case .s: adjustedValue = value / 1000_000_000.0
+                        }
+                    case .inverseTime:
+                        switch result.settings.inverseTimeUnit {
+                        case .ns: adjustedValue = value
+                        case .us: adjustedValue = value * 1000.0
+                        case .ms: adjustedValue = value * 1000_000.0
+                        case .s: adjustedValue = value * 1000_000_000.0
+                        }
+                    case .none:
+                        adjustedValue = value
+                    }
                     if pretty {
-                        content = column.formatter(value, result.settings)
+                        content = column.formatter(adjustedValue, result.settings)
                     } else {
-                        content = String(value)
+                        content = String(adjustedValue)
                     }
                 }
                 row[column] = content
diff --git a/Sources/Benchmark/BenchmarkFormatter.swift b/Sources/Benchmark/BenchmarkFormatter.swift
index 63df44c..3736571 100644
--- a/Sources/Benchmark/BenchmarkFormatter.swift
+++ b/Sources/Benchmark/BenchmarkFormatter.swift
@@ -28,22 +28,12 @@ public enum BenchmarkFormatter {
 
     /// Show number with the corresponding time unit.
     public static let time: Formatter = { (value, settings) in
-        switch settings.timeUnit {
-        case .ns: return "\(value) ns"
-        case .us: return "\(value/1000.0) us"
-        case .ms: return "\(value/1000_000.0) ms"
-        case .s: return "\(value/1000_000_000.0) s"
-        }
+        return "\(value) \(settings.timeUnit)"
     }
 
     /// Show number with the corresponding inverse time unit.
     public static let inverseTime: Formatter = { (value, settings) in
-        switch settings.inverseTimeUnit {
-        case .ns: return "\(value) /ns"
-        case .us: return "\(value*1000.0) /us"
-        case .ms: return "\(value*1000_000.0) /ms"
-        case .s: return "\(value*1000_000_000.0) /s"
-        }
+        return "\(value) /\(settings.inverseTimeUnit)"
     }
 
     /// Show value as percentage.
diff --git a/Sources/Benchmark/BenchmarkMain.swift b/Sources/Benchmark/BenchmarkMain.swift
index f92f784..431d2e6 100644
--- a/Sources/Benchmark/BenchmarkMain.swift
+++ b/Sources/Benchmark/BenchmarkMain.swift
@@ -12,18 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-public func main(_ suites: [BenchmarkSuite]) {
-    let command = BenchmarkCommand.parseOrExit()
-    main(suites, settings: command.arguments.settings)
-}
-
-public func main(_ suites: [BenchmarkSuite], settings: [BenchmarkSetting]) {
+public func main(
+    _ suites: [BenchmarkSuite] = [defaultBenchmarkSuite],
+    settings: [BenchmarkSetting] = parseArguments(),
+    customDefaults: [BenchmarkSetting] = defaultSettings
+) {
     var runner = BenchmarkRunner(
         suites: suites,
-        settings: settings)
+        settings: settings,
+        customDefaults: customDefaults)
     try! runner.run()
 }
-
-public func main() {
-    main([defaultBenchmarkSuite])
-}
diff --git a/Sources/Benchmark/BenchmarkRunner.swift b/Sources/Benchmark/BenchmarkRunner.swift
index 42f1e13..4cac1ac 100644
--- a/Sources/Benchmark/BenchmarkRunner.swift
+++ b/Sources/Benchmark/BenchmarkRunner.swift
@@ -15,16 +15,23 @@
 public struct BenchmarkRunner {
     let suites: [BenchmarkSuite]
     let settings: [BenchmarkSetting]
+    let customDefaults: [BenchmarkSetting]
     let globalSettings: BenchmarkSettings
     var progress: ProgressReporter
     var reporter: BenchmarkReporter
     var results: [BenchmarkResult] = []
 
-    init(suites: [BenchmarkSuite], settings: [BenchmarkSetting]) {
+    init(
+        suites: [BenchmarkSuite],
+        settings: [BenchmarkSetting],
+        customDefaults: [BenchmarkSetting] = []
+    ) {
         self.suites = suites
         self.settings = settings
+        self.customDefaults = customDefaults
         self.globalSettings = BenchmarkSettings([
             defaultSettings,
+            self.customDefaults,
             self.settings,
         ])
         switch self.globalSettings.format {
@@ -61,6 +68,7 @@ public struct BenchmarkRunner {
     mutating func run(benchmark: AnyBenchmark, suite: BenchmarkSuite) throws {
         let settings = BenchmarkSettings([
             defaultSettings,
+            self.customDefaults,
             suite.settings,
             benchmark.settings,
             self.settings,
diff --git a/Sources/Benchmark/BenchmarkSetting.swift b/Sources/Benchmark/BenchmarkSetting.swift
index 42097ac..6bdbcfc 100644
--- a/Sources/Benchmark/BenchmarkSetting.swift
+++ b/Sources/Benchmark/BenchmarkSetting.swift
@@ -226,7 +226,7 @@ public struct BenchmarkSettings {
     }
 }
 
-let defaultSettings: [BenchmarkSetting] = [
+public let defaultSettings: [BenchmarkSetting] = [
     MaxIterations(1_000_000),
     MinTime(seconds: 1.0),
     TimeUnit(.ns),
diff --git a/Sources/Benchmark/BenchmarkSuite.swift b/Sources/Benchmark/BenchmarkSuite.swift
index dda724b..e3ada7f 100644
--- a/Sources/Benchmark/BenchmarkSuite.swift
+++ b/Sources/Benchmark/BenchmarkSuite.swift
@@ -87,4 +87,4 @@ public class BenchmarkSuite {
     }
 }
 
-var defaultBenchmarkSuite = BenchmarkSuite(name: "")
+public let defaultBenchmarkSuite = BenchmarkSuite(name: "")


Test patch:
diff --git a/Tests/BenchmarkTests/BenchmarkReporterTests.swift b/Tests/BenchmarkTests/BenchmarkReporterTests.swift
index a4647dd..dfed2f4 100644
--- a/Tests/BenchmarkTests/BenchmarkReporterTests.swift
+++ b/Tests/BenchmarkTests/BenchmarkReporterTests.swift
@@ -82,7 +82,7 @@ final class BenchmarkReporterTests: XCTestCase {
         assertConsoleReported(results, expected)
     }
 
-    func testConsoleCountersAreReported() throws {
+    func testConsoleCounters() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
                 benchmarkName: "fast", suiteName: "MySuite",
@@ -106,7 +106,7 @@ final class BenchmarkReporterTests: XCTestCase {
         assertConsoleReported(results, expected)
     }
 
-    func testConsoleWarmupReported() throws {
+    func testConsoleWarmup() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
                 benchmarkName: "fast", suiteName: "MySuite",
@@ -130,7 +130,7 @@ final class BenchmarkReporterTests: XCTestCase {
         assertConsoleReported(results, expected)
     }
 
-    func testConsoleTimeUnitReported() throws {
+    func testConsoleTimeUnit() throws {
         let results: [BenchmarkResult] = [
             BenchmarkResult(
                 benchmarkName: "ns", suiteName: "MySuite",
@@ -248,6 +248,59 @@ final class BenchmarkReporterTests: XCTestCase {
         assertJSONReported(results, expected, settings: settings)
     }
 
+    func testJSONTimeUnit() throws {
+        let results: [BenchmarkResult] = [
+            BenchmarkResult(
+                benchmarkName: "ns", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.ns)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+            BenchmarkResult(
+                benchmarkName: "us", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.us)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+            BenchmarkResult(
+                benchmarkName: "ms", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.ms)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+            BenchmarkResult(
+                benchmarkName: "s", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.s)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+        ]
+        let expected = #"""
+            {
+              "benchmarks": [
+                {
+                  "name": "MySuite.ns",
+                  "time": 123456789.0
+                },
+                {
+                  "name": "MySuite.us",
+                  "time": 123456.789
+                },
+                {
+                  "name": "MySuite.ms",
+                  "time": 123.456789
+                },
+                {
+                  "name": "MySuite.s",
+                  "time": 0.123456789
+                }
+              ]
+            }
+            """#
+        let settings = BenchmarkSettings([Columns(["name", "time"])])
+        assertJSONReported(results, expected, settings: settings)
+    }
+
     func testCSVEmpty() {
         let results: [BenchmarkResult] = []
         let expected = #"""
@@ -301,16 +354,56 @@ final class BenchmarkReporterTests: XCTestCase {
         assertCSVReported(results, expected, settings: settings)
     }
 
+    func testCSVTimeUnit() throws {
+        let results: [BenchmarkResult] = [
+            BenchmarkResult(
+                benchmarkName: "ns", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.ns)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+            BenchmarkResult(
+                benchmarkName: "us", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.us)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+            BenchmarkResult(
+                benchmarkName: "ms", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.ms)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+            BenchmarkResult(
+                benchmarkName: "s", suiteName: "MySuite",
+                settings: BenchmarkSettings([TimeUnit(.s)]),
+                measurements: [123_456_789],
+                warmupMeasurements: [],
+                counters: [:]),
+        ]
+        let expected = #"""
+            name,time
+            MySuite.ns,123456789.0
+            MySuite.us,123456.789
+            MySuite.ms,123.456789
+            MySuite.s,0.123456789
+            """#
+        let settings = BenchmarkSettings([Columns(["name", "time"])])
+        assertCSVReported(results, expected, settings: settings)
+    }
+
     static var allTests = [
         ("testConsoleBasic", testConsoleBasic),
-        ("testConsoleCountersAreReported", testConsoleCountersAreReported),
-        ("testConsoleWarmupReported", testConsoleWarmupReported),
-        ("testConsoleTimeUnitReported", testConsoleTimeUnitReported),
+        ("testConsoleCounters", testConsoleCounters),
+        ("testConsoleWarmup", testConsoleWarmup),
+        ("testConsoleTimeUnit", testConsoleTimeUnit),
         ("testJSONEmpty", testJSONEmpty),
         ("testJSONBasic", testJSONBasic),
         ("testJSONEscape", testJSONEscape),
+        ("testJSONTimeUnit", testJSONTimeUnit),
         ("testCSVEmpty", testCSVEmpty),
         ("testCSVBasic", testCSVBasic),
         ("testCSVEscape", testCSVEscape),
+        ("testCSVTimeUnit", testCSVTimeUnit),
     ]
 }
diff --git a/Tests/BenchmarkTests/BenchmarkSettingTests.swift b/Tests/BenchmarkTests/BenchmarkSettingTests.swift
index 8551a6a..833020a 100644
--- a/Tests/BenchmarkTests/BenchmarkSettingTests.swift
+++ b/Tests/BenchmarkTests/BenchmarkSettingTests.swift
@@ -21,11 +21,13 @@ final class BenchmarkSettingTests: XCTestCase {
     func assertNumberOfIterations(
         suite: BenchmarkSuite,
         counts expected: [Int],
-        cli: [BenchmarkSetting]
+        cli: [BenchmarkSetting],
+        customDefaults: [BenchmarkSetting] = []
     ) throws {
         var settings: [BenchmarkSetting] = [Format(.none)]
         settings.append(contentsOf: cli)
-        var runner = BenchmarkRunner(suites: [suite], settings: settings)
+        var runner = BenchmarkRunner(
+            suites: [suite], settings: settings, customDefaults: customDefaults)
 
         try runner.run()
         XCTAssertEqual(runner.results.count, expected.count)
@@ -124,6 +126,54 @@ final class BenchmarkSettingTests: XCTestCase {
             cli: [Iterations(1)])
     }
 
+    func testCustomDefaults() throws {
+        let suite = BenchmarkSuite(name: "Test") { suite in
+            suite.benchmark("a") {}
+            suite.benchmark("b") {}
+        }
+        try assertNumberOfIterations(
+            suite: suite,
+            counts: [1, 1],
+            cli: [],
+            customDefaults: [Iterations(1)])
+    }
+
+    func testCustomDafaultsOverridenBySuite() throws {
+        let suite = BenchmarkSuite(name: "Test", settings: Iterations(3)) { suite in
+            suite.benchmark("a") {}
+            suite.benchmark("b") {}
+        }
+        try assertNumberOfIterations(
+            suite: suite,
+            counts: [3, 3],
+            cli: [],
+            customDefaults: [Iterations(1)])
+    }
+
+    func testCustomDafaultsOverridenByBenchmark() throws {
+        let suite = BenchmarkSuite(name: "Test", settings: Iterations(3)) { suite in
+            suite.benchmark("a") {}
+            suite.benchmark("b", settings: Iterations(4)) {}
+        }
+        try assertNumberOfIterations(
+            suite: suite,
+            counts: [3, 4],
+            cli: [],
+            customDefaults: [Iterations(1)])
+    }
+
+    func testCustomDafaultsOverridenByCli() throws {
+        let suite = BenchmarkSuite(name: "Test", settings: Iterations(3)) { suite in
+            suite.benchmark("a") {}
+            suite.benchmark("b", settings: Iterations(4)) {}
+        }
+        try assertNumberOfIterations(
+            suite: suite,
+            counts: [5, 5],
+            cli: [Iterations(5)],
+            customDefaults: [Iterations(1)])
+    }
+
     static var allTests = [
         ("testDefaultSetting", testDefaultSetting),
         ("testSuiteSetting", testSuiteSetting),
@@ -133,5 +183,9 @@ final class BenchmarkSettingTests: XCTestCase {
         ("testCliOverridesSuiteSetting", testCliOverridesSuiteSetting),
         ("testCliOverridesBenchmarkSetting", testCliOverridesBenchmarkSetting),
         ("testCliOverridesBenchmarkAndSuiteSetting", testCliOverridesBenchmarkAndSuiteSetting),
+        ("testCustomDefaults", testCustomDefaults),
+        ("testCustomDafaultsOverridenBySuite", testCustomDafaultsOverridenBySuite),
+        ("testCustomDafaultsOverridenByBenchmark", testCustomDafaultsOverridenByBenchmark),
+        ("testCustomDafaultsOverridenByCli", testCustomDafaultsOverridenByCli),
     ]
 }