-
Notifications
You must be signed in to change notification settings - Fork 5
fix: add compatibility for Zig 0.16 ArrayList and timer changes #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,28 +16,45 @@ const pack = msgpack.Pack( | |
| bufferType.read, | ||
| ); | ||
|
|
||
| const is_zig_16 = builtin.zig_version.minor >= 16; | ||
| const BenchRuntime = if (is_zig_16) struct { | ||
| var io: ?std.Io = null; | ||
| } else struct {}; | ||
|
|
||
| /// Get monotonic time in nanoseconds on Zig 0.16+. | ||
| fn getTimeNs() u64 { | ||
| if (is_zig_16) { | ||
| const io = BenchRuntime.io orelse @panic("benchmark runtime io is not initialized"); | ||
| return @intCast(std.Io.Clock.awake.now(io).nanoseconds); | ||
| } | ||
|
|
||
| unreachable; | ||
| } | ||
|
Comment on lines
+25
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The manual implementation of |
||
|
|
||
| /// Benchmark timer helper | ||
| /// Run a benchmark and print results | ||
| fn benchmark( | ||
| comptime name: []const u8, | ||
| comptime iterations: usize, | ||
| comptime func: fn (allocator: std.mem.Allocator) anyerror!void, | ||
| ) !void { | ||
| var gpa = std.heap.GeneralPurposeAllocator(.{}){}; | ||
| defer _ = gpa.deinit(); | ||
| const allocator = gpa.allocator(); | ||
| const allocator = std.heap.page_allocator; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
|
|
||
| // Warmup | ||
| for (0..10) |_| { | ||
| try func(allocator); | ||
| } | ||
|
|
||
| // Actual benchmark | ||
| var timer = try std.time.Timer.start(); | ||
| var legacy_timer: if (is_zig_16) void else std.time.Timer = if (is_zig_16) {} else undefined; | ||
| if (!is_zig_16) legacy_timer = try std.time.Timer.start(); | ||
| const start_ns = if (is_zig_16) getTimeNs() else 0; | ||
|
Comment on lines
+49
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The conditional logic for handling the timer across Zig versions is quite clunky. Instead of manually implementing |
||
|
|
||
| for (0..iterations) |_| { | ||
| try func(allocator); | ||
| } | ||
| const elapsed_ns = timer.read(); | ||
|
|
||
| const elapsed_ns = if (is_zig_16) getTimeNs() - start_ns else legacy_timer.read(); | ||
|
|
||
| const avg_ns = elapsed_ns / iterations; | ||
| const ops_per_sec = if (avg_ns > 0) (1_000_000_000 / avg_ns) else 0; | ||
|
|
@@ -850,7 +867,7 @@ fn benchMixedTypesRead(allocator: std.mem.Allocator) !void { | |
| // Main Benchmark Runner | ||
| // ============================================================================ | ||
|
|
||
| pub fn main() !void { | ||
| fn runBenchmarks() !void { | ||
| std.debug.print("\n", .{}); | ||
| std.debug.print("=" ** 80 ++ "\n", .{}); | ||
| std.debug.print("MessagePack Benchmark Suite\n", .{}); | ||
|
|
@@ -936,3 +953,16 @@ pub fn main() !void { | |
| std.debug.print("Benchmark Complete\n", .{}); | ||
| std.debug.print("=" ** 80 ++ "\n", .{}); | ||
| } | ||
|
|
||
| const BenchEntry = if (is_zig_16) struct { | ||
| pub fn main(init: std.process.Init) !void { | ||
| BenchRuntime.io = init.io; | ||
| try runBenchmarks(); | ||
| } | ||
| } else struct { | ||
| pub fn main() !void { | ||
| try runBenchmarks(); | ||
| } | ||
| }; | ||
|
|
||
| pub const main = BenchEntry.main; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1316,7 +1316,7 @@ pub const Payload = union(enum) { | |
| var new_heap = if (current_zig.minor == 14) | ||
| std.ArrayList(Payload).init(alloc) | ||
| else | ||
| std.ArrayList(Payload){}; | ||
| std.ArrayList(Payload).initCapacity(alloc, 0) catch unreachable; | ||
|
Comment on lines
1316
to
+1319
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
|
|
||
| // Copy existing items from stack buffer to heap | ||
| for (buffer[0..len.*]) |item| { | ||
|
|
@@ -2857,7 +2857,7 @@ pub fn PackWithLimits( | |
| var parse_stack = if (current_zig.minor == 14) | ||
| std.ArrayList(ParseState).init(allocator) | ||
| else | ||
| std.ArrayList(ParseState){}; | ||
| try std.ArrayList(ParseState).initCapacity(allocator, 0); | ||
|
Comment on lines
2857
to
+2860
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the previous comment, |
||
| defer if (current_zig.minor == 14) parse_stack.deinit() else parse_stack.deinit(allocator); | ||
| errdefer cleanupParseStack(&parse_stack, allocator); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The claim that
std.heap.GeneralPurposeAllocatoris removed in Zig 0.16 is incorrect. It remains a core part of the standard library, although its internal implementation details or its location withinstd.heapmay have evolved. Switching topage_allocatorin benchmarks is a regression in measurement quality as it is significantly slower due to per-allocation system calls.