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.

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,

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

@@ -314,7 +314,7 @@ 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,
@@ -325,13 +325,11 @@ pub const Result = struct {
         name: []const u8,
         readings: Runner.Readings,
     ) !Result {
-        var r = Result{
+        return Result{
             .allocator = allocator,
             .name = name,
             .readings = readings,
         };
-        r.readings.sort();
-        return r;
     }
 
     pub fn deinit(self: Result) void {
@@ -344,7 +342,8 @@ pub const Result = struct {
     pub fn prettyPrint(self: Result, 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(self.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 +379,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(self.allocator, allocs.maxes);
             // Benchmark name
             const name = try std.fmt.bufPrint(&buf, "{s} [MEMORY]", .{
                 self.name,
@@ -420,9 +419,11 @@ pub const Result = struct {
     }
 
     pub fn writeJSON(self: Result, writer: anytype) !void {
-        const timings_ns_stats = Statistics(u64).init(self.readings.timings_ns);
+        const timings_ns_stats =
+            try Statistics(u64).init(self.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(self.allocator, allocs.maxes);
             try writer.print(
                 \\{{ "name": "{s}",
                 \\   "timing_statistics": {}, "timings": {},