Don't mutate the readings when calculating statistics (#73)

* Don't mutate the readings when calculating statistics

There isn't much cost to making a temporary copy of them for sorting, IMHO, and
it preserves the link between the readings for each run.

* Provide an allocator to prettyPrint and writeJSON

So `Results` doesn't need to carry one, and then we can use an arena for pretty
printing allocations more easily.

examples/json.zig

@@ -35,7 +35,7 @@ test "bench test json" {
             defer x.deinit();
             defer i += 1;
             if (0 < i) try stdout.writeAll(", ");
-            try x.writeJSON(stdout);
+            try x.writeJSON(test_allocator, stdout);
         },
     };
     try stdout.writeAll("]\n");

examples/memory_tracking.zig

@@ -8,7 +8,7 @@ fn myBenchmark(allocator: std.mem.Allocator) void {
     }
 }
 
-test "bench test basic" {
+test "bench test memory tracking" {
     const stdout = std.io.getStdOut().writer();
     var bench = zbench.Benchmark.init(std.testing.allocator, .{
         .iterations = 64,

examples/progress.zig

@@ -46,7 +46,7 @@ test "bench test progress" {
             progress_node.setEstimatedTotalItems(0);
             progress_node.setCompletedItems(0);
             progress.refresh();
-            try x.prettyPrint(stdout, true);
+            try x.prettyPrint(test_allocator, stdout, true);
         },
     };
 }

util/runner/types.zig

@@ -58,14 +58,6 @@ pub const Readings = struct {
             }
         }
     }
-
-    pub fn sort(self: *Readings) void {
-        std.sort.heap(u64, self.timings_ns, {}, std.sort.asc(u64));
-        if (self.allocations) |allocs| {
-            std.sort.heap(usize, allocs.maxes, {}, std.sort.asc(usize));
-            std.sort.heap(usize, allocs.counts, {}, std.sort.asc(usize));
-        }
-    }
 };
 
 pub const AllocationReading = struct {

util/statistics.zig

@@ -18,9 +18,8 @@ pub fn Statistics(comptime T: type) type {
             p995: T,
         };
 
-        /// Create a statistical summary of a dataset, NB. assumes that the
-        /// readings are sorted.
-        pub fn init(readings: []const T) Self {
+        /// Create a statistical summary of a dataset.
+        pub fn init(allocator: std.mem.Allocator, readings: []const T) !Self {
             const len = readings.len;
 
             // Calculate total and mean
@@ -38,16 +37,20 @@ pub fn Statistics(comptime T: type) type {
                 break :blk if (1 < len) std.math.sqrt(nvar / (len - 1)) else 0;
             };
 
+            const sorted = try allocator.dupe(T, readings);
+            defer allocator.free(sorted);
+            std.sort.heap(T, sorted, {}, std.sort.asc(T));
+
             return Self{
                 .total = total,
                 .mean = mean,
                 .stddev = stddev,
-                .min = if (len == 0) 0 else readings[0],
-                .max = if (len == 0) 0 else readings[len - 1],
+                .min = if (len == 0) 0 else sorted[0],
+                .max = if (len == 0) 0 else sorted[len - 1],
                 .percentiles = Percentiles{
-                    .p75 = if (len == 0) 0 else readings[len * 75 / 100],
-                    .p99 = if (len == 0) 0 else readings[len * 99 / 100],
-                    .p995 = if (len == 0) 0 else readings[len * 995 / 1000],
+                    .p75 = if (len == 0) 0 else sorted[len * 75 / 100],
+                    .p99 = if (len == 0) 0 else sorted[len * 99 / 100],
+                    .p995 = if (len == 0) 0 else sorted[len * 995 / 1000],
                 },
             };
         }
@@ -109,7 +112,7 @@ test "Statistics" {
                 .p99 = 0,
                 .p995 = 0,
             },
-        }, Statistics(u64).init(timings_ns.items));
+        }, try Statistics(u64).init(std.testing.allocator, timings_ns.items));
     }
 
     {
@@ -127,7 +130,7 @@ test "Statistics" {
                 .p99 = 1,
                 .p995 = 1,
             },
-        }, Statistics(u64).init(timings_ns.items));
+        }, try Statistics(u64).init(std.testing.allocator, timings_ns.items));
     }
 
     {
@@ -146,14 +149,34 @@ test "Statistics" {
                 .p99 = 15,
                 .p995 = 15,
             },
-        }, Statistics(u64).init(timings_ns.items));
+        }, try Statistics(u64).init(std.testing.allocator, timings_ns.items));
+    }
+
+    {
+        var timings_ns = std.ArrayList(u64).init(std.testing.allocator);
+        defer timings_ns.deinit();
+        try timings_ns.append(1);
+        for (1..101) |i| try timings_ns.append(i);
+        try expectEqDeep(Statistics(u64){
+            .total = 5051,
+            .mean = 50,
+            .stddev = 29,
+            .min = 1,
+            .max = 100,
+            .percentiles = .{
+                .p75 = 75,
+                .p99 = 99,
+                .p995 = 100,
+            },
+        }, try Statistics(u64).init(std.testing.allocator, timings_ns.items));
     }
 
     {
         var timings_ns = std.ArrayList(u64).init(std.testing.allocator);
         defer timings_ns.deinit();
         try timings_ns.append(1);
         for (1..101) |i| try timings_ns.append(i);
+        std.mem.reverse(u64, timings_ns.items);
         try expectEqDeep(Statistics(u64){
             .total = 5051,
             .mean = 50,
@@ -165,6 +188,6 @@ test "Statistics" {
                 .p99 = 99,
                 .p995 = 100,
             },
-        }, Statistics(u64).init(timings_ns.items));
+        }, try Statistics(u64).init(std.testing.allocator, timings_ns.items));
     }
 }

zbench.zig

@@ -234,7 +234,6 @@ pub const Benchmark = struct {
                 defer self.remaining = self.remaining[1..];
                 if (self.remaining[0].config.hooks.after_all) |hook| hook();
                 return Step{ .result = try Result.init(
-                    self.allocator,
                     self.remaining[0].name,
                     try runner.finish(),
                 ) };
@@ -267,6 +266,11 @@ pub const Benchmark = struct {
         const progress_node = progress.start("", 0);
         defer progress_node.end();
 
+        // Most allocations for pretty printing will be the same size each time,
+        // so using an arena should reduce the allocation load.
+        var arena = std.heap.ArenaAllocator.init(self.allocator);
+        defer arena.deinit();
+
         try prettyPrintHeader(writer);
         var iter = try self.iterator();
         while (try iter.next()) |step| switch (step) {
@@ -282,7 +286,8 @@ pub const Benchmark = struct {
                 progress_node.setEstimatedTotalItems(0);
                 progress_node.setCompletedItems(0);
                 progress.refresh();
-                try x.prettyPrint(writer, true);
+                try x.prettyPrint(arena.allocator(), writer, true);
+                _ = arena.reset(.retain_capacity);
             },
         };
     }
@@ -314,24 +319,13 @@ pub fn getSystemInfo() !platform.OsInfo {
 
 /// Carries the results of a benchmark. The benchmark name and the recorded
 /// durations are available, and some basic statistics are automatically
-/// calculated. The timings can always be assumed to be sorted.
+/// calculated.
 pub const Result = struct {
-    allocator: std.mem.Allocator,
     name: []const u8,
     readings: Readings,
 
-    pub fn init(
-        allocator: std.mem.Allocator,
-        name: []const u8,
-        readings: Runner.Readings,
-    ) !Result {
-        var r = Result{
-            .allocator = allocator,
-            .name = name,
-            .readings = readings,
-        };
-        r.readings.sort();
-        return r;
+    pub fn init(name: []const u8, readings: Runner.Readings) !Result {
+        return Result{ .name = name, .readings = readings };
     }
 
     pub fn deinit(self: Result) void {
@@ -341,10 +335,16 @@ pub const Result = struct {
     /// Formats and prints the benchmark result in a human readable format.
     /// writer: Type that has the associated method print (for example std.io.getStdOut.writer())
     /// colors: Whether to pretty-print with ANSI colors or not.
-    pub fn prettyPrint(self: Result, writer: anytype, colors: bool) !void {
+    pub fn prettyPrint(
+        self: Result,
+        allocator: std.mem.Allocator,
+        writer: anytype,
+        colors: bool,
+    ) !void {
         var buf: [128]u8 = undefined;
 
-        const s = Statistics(u64).init(self.readings.timings_ns);
+        const timings_ns = self.readings.timings_ns;
+        const s = try Statistics(u64).init(allocator, timings_ns);
         // Benchmark name, number of iterations, and total time
         try writer.print("{s:<22} ", .{self.name});
         try setColor(colors, writer, Color.cyan);
@@ -380,7 +380,7 @@ pub const Result = struct {
         try writer.writeAll("\n");
 
         if (self.readings.allocations) |allocs| {
-            const m = Statistics(usize).init(allocs.maxes);
+            const m = try Statistics(usize).init(allocator, allocs.maxes);
             // Benchmark name
             const name = try std.fmt.bufPrint(&buf, "{s} [MEMORY]", .{
                 self.name,
@@ -419,10 +419,16 @@ pub const Result = struct {
         if (colors) try writer.writeAll(color.code());
     }
 
-    pub fn writeJSON(self: Result, writer: anytype) !void {
-        const timings_ns_stats = Statistics(u64).init(self.readings.timings_ns);
+    pub fn writeJSON(
+        self: Result,
+        allocator: std.mem.Allocator,
+        writer: anytype,
+    ) !void {
+        const timings_ns_stats =
+            try Statistics(u64).init(allocator, self.readings.timings_ns);
         if (self.readings.allocations) |allocs| {
-            const allocation_maxes_stats = Statistics(usize).init(allocs.maxes);
+            const allocation_maxes_stats =
+                try Statistics(usize).init(allocator, allocs.maxes);
             try writer.print(
                 \\{{ "name": "{s}",
                 \\   "timing_statistics": {}, "timings": {},