From fdb2ac52d02a8283740e84d17219e15e526fb92e Mon Sep 17 00:00:00 2001 From: Ben Sinclair Date: Thu, 11 Apr 2024 12:00:49 +1000 Subject: [PATCH] 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 | 2 +- util/runner/types.zig | 8 ------ util/statistics.zig | 47 +++++++++++++++++++++++++++--------- zbench.zig | 17 +++++++------ 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/examples/memory_tracking.zig b/examples/memory_tracking.zig index 21c41be..b933c7d 100644 --- a/examples/memory_tracking.zig +++ b/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, diff --git a/util/runner/types.zig b/util/runner/types.zig index f09742a..bd9b520 100644 --- a/util/runner/types.zig +++ b/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 { diff --git a/util/statistics.zig b/util/statistics.zig index 7be963b..1895d16 100644 --- a/util/statistics.zig +++ b/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,7 +149,26 @@ 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)); } { @@ -154,6 +176,7 @@ test "Statistics" { 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)); } } diff --git a/zbench.zig b/zbench.zig index fbb8e34..f95276d 100644 --- a/zbench.zig +++ b/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": {},