From 1af686e16174982d2a180bc5fa656ea98a72beac Mon Sep 17 00:00:00 2001 From: dweiller <4678790+dweiller@users.noreply.github.com> Date: Tue, 3 Sep 2024 03:29:50 +1000 Subject: [PATCH] chore!: remove usage of `@setCold` The `@setCold` builtin was removed in 0.14.0-dev.1337+d3c6f7179 and replaced with `@branchHint`. Unfortunately comptime conditional logic cannot be used to choose between `@setCold` and `@branchHint`, so separate logging modules are chosen between at build time. Other usages of `@setCold` have been removed without replacement. --- build.zig | 27 ++++++++++ src/Heap.zig | 13 ++--- src/Page.zig | 2 +- src/ThreadHeapMap.zig | 2 +- src/allocator.zig | 2 +- src/assert.zig | 2 +- src/libzimalloc.zig | 2 +- src/log_branchHint.zig | 89 ++++++++++++++++++++++++++++++++ src/{log.zig => log_setCold.zig} | 0 9 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 src/log_branchHint.zig rename src/{log.zig => log_setCold.zig} (100%) diff --git a/build.zig b/build.zig index d323eab..26c8100 100644 --- a/build.zig +++ b/build.zig @@ -33,10 +33,32 @@ pub fn build(b: *std.Build) void { build_options.addOption(bool, "panic_on_invalid", panic_on_invalid); const zimalloc_options = build_options.createModule(); + + const branch_hint_version: std.SemanticVersion = .{ + .major = 0, + .minor = 14, + .patch = 0, + .pre = "0.14.0-dev.1402+cad65307b", + .build = "0.14.0-dev.1402+cad65307b", + }; + + const log_module_source = if (@import("builtin").zig_version.order(branch_hint_version) == .lt) + "src/log_setCold.zig" + else + "src/log_branchHint.zig"; + + const log_module = b.createModule(.{ + .root_source_file = b.path(log_module_source), + .imports = &.{ + .{ .name = "build_options", .module = zimalloc_options }, + }, + }); + const zimalloc = b.addModule("zimalloc", .{ .root_source_file = b.path("src/zimalloc.zig"), .imports = &.{ .{ .name = "build_options", .module = zimalloc_options }, + .{ .name = "log", .module = log_module }, }, }); @@ -46,6 +68,7 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = optimize, .zimalloc_options = zimalloc_options, + .log_module = log_module, }); const libzimalloc_install = b.addInstallArtifact(libzimalloc, .{}); @@ -66,6 +89,7 @@ pub fn build(b: *std.Build) void { .target = target, .optimize = config.optimize, .zimalloc_options = options_module, + .log_module = log_module, }); const install = b.addInstallArtifact(compile, .{ @@ -85,6 +109,7 @@ pub fn build(b: *std.Build) void { .link_libc = true, }); tests.root_module.addImport("build_options", zimalloc_options); + tests.root_module.addImport("log", log_module); const tests_run = b.addRunArtifact(tests); @@ -132,6 +157,7 @@ const LibzimallocOptions = struct { zimalloc_options: *std.Build.Module, linkage: std.builtin.LinkMode = .dynamic, pic: ?bool = true, + log_module: *std.Build.Module, }; fn addLibzimalloc(b: *std.Build, options: LibzimallocOptions) *std.Build.Step.Compile { @@ -158,6 +184,7 @@ fn addLibzimalloc(b: *std.Build, options: LibzimallocOptions) *std.Build.Step.Co }; libzimalloc.root_module.addImport("build_options", options.zimalloc_options); + libzimalloc.root_module.addImport("log", options.log_module); return libzimalloc; } diff --git a/src/Heap.zig b/src/Heap.zig index 80533ba..c53357a 100644 --- a/src/Heap.zig +++ b/src/Heap.zig @@ -53,7 +53,7 @@ pub fn allocateSizeClass(self: *Heap, class: usize, log2_align: u8) ?[*]align(co return @ptrFromInt(aligned_address); } - if (isNullPageNode(page_node)) unlikely() else { + if (!isNullPageNode(page_node)) { page_node.data.migrateFreeList(); } @@ -65,7 +65,7 @@ pub fn allocateSizeClass(self: *Heap, class: usize, log2_align: u8) ?[*]align(co log.debugVerbose("alloc slow path", .{}); const slot = slot: { - if (isNullPageNode(page_node)) unlikely() else { + if (!isNullPageNode(page_node)) { var node = page_node.next; var prev = page_node; while (node != page_node) { @@ -235,7 +235,6 @@ fn initPage(self: *Heap, class: usize) error{OutOfMemory}!*Page.List.Node { if (isNullPageNode(self.pages[class].head.?)) { // capcity == 0 means it's the null page - unlikely(); self.pages[class].head = page_node; } else { self.pages[class].prependOne(page_node); @@ -287,18 +286,12 @@ fn isNullPageNode(page_node: *const Page.List.Node) bool { return page_node == &null_page_list_node; } -// TODO: replace this attempted workaround when https://github.com/ziglang/zig/issues/5177 -// gets implemented -fn unlikely() void { - @setCold(true); -} - const size_class_count = size_class.count; const std = @import("std"); const assert = @import("assert.zig"); -const log = @import("log.zig"); +const log = @import("log"); const constants = @import("constants.zig"); const size_class = @import("size_class.zig"); diff --git a/src/Page.zig b/src/Page.zig index 427393e..c79af5b 100644 --- a/src/Page.zig +++ b/src/Page.zig @@ -183,6 +183,6 @@ const std = @import("std"); const assert = @import("assert.zig"); const constants = @import("constants.zig"); const list = @import("list.zig"); -const log = @import("log.zig"); +const log = @import("log"); const Segment = @import("Segment.zig"); diff --git a/src/ThreadHeapMap.zig b/src/ThreadHeapMap.zig index d53eccc..277537f 100644 --- a/src/ThreadHeapMap.zig +++ b/src/ThreadHeapMap.zig @@ -117,5 +117,5 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const Heap = @import("Heap.zig"); -const log = @import("log.zig"); +const log = @import("log"); const list = @import("list.zig"); diff --git a/src/allocator.zig b/src/allocator.zig index be49a15..e8b95cc 100644 --- a/src/allocator.zig +++ b/src/allocator.zig @@ -375,7 +375,7 @@ const builtin = @import("builtin"); const assert = @import("assert.zig"); const constants = @import("constants.zig"); -const log = @import("log.zig"); +const log = @import("log"); const huge_alignment = @import("huge_alignment.zig"); const Heap = @import("Heap.zig"); diff --git a/src/assert.zig b/src/assert.zig index 6a75769..137c24f 100644 --- a/src/assert.zig +++ b/src/assert.zig @@ -12,4 +12,4 @@ pub fn withMessage(src_loc: std.builtin.SourceLocation, ok: bool, message: []con } const std = @import("std"); -const log = @import("log.zig"); +const log = @import("log"); diff --git a/src/libzimalloc.zig b/src/libzimalloc.zig index 6c5a96a..da719e8 100644 --- a/src/libzimalloc.zig +++ b/src/libzimalloc.zig @@ -159,7 +159,7 @@ const std = @import("std"); const zimalloc = @import("zimalloc.zig"); const assert = @import("assert.zig"); -const log = @import("log.zig"); +const log = @import("log"); const constants = @import("constants.zig"); const build_options = @import("build_options"); diff --git a/src/log_branchHint.zig b/src/log_branchHint.zig new file mode 100644 index 0000000..c64cebf --- /dev/null +++ b/src/log_branchHint.zig @@ -0,0 +1,89 @@ +/// Log an error message. This log level is intended to be used +/// when something has gone wrong. This might be recoverable or might +/// be followed by the program exiting. +pub fn err( + comptime format: []const u8, + args: anytype, +) void { + @branchHint(.cold); + log(.err, format, args); +} + +/// Log a warning message. This log level is intended to be used if +/// it is uncertain whether something has gone wrong or not, but the +/// circumstances would be worth investigating. +pub fn warn( + comptime format: []const u8, + args: anytype, +) void { + log(.warn, format, args); +} + +/// Log an info message. This log level is intended to be used for +/// general messages about the state of the program. +pub fn info( + comptime format: []const u8, + args: anytype, +) void { + log(.info, format, args); +} + +/// Log an info message. This log level is intended to be used for +/// general messages about the state of the program that are noisy +/// and are turned off by default. +pub fn infoVerbose( + comptime format: []const u8, + args: anytype, +) void { + if (comptime !verbose_logging) return; + log(.info, format, args); +} + +/// Log a debug message. This log level is intended to be used for +/// messages which are only useful for debugging. +pub fn debug( + comptime format: []const u8, + args: anytype, +) void { + log(.debug, format, args); +} + +/// Log a debug message. This log level is intended to be used for +/// messages which are only useful for debugging that are noisy and +/// are turned off by default. +pub fn debugVerbose( + comptime format: []const u8, + args: anytype, +) void { + if (comptime !verbose_logging) return; + log(.debug, format, args); +} + +const verbose_logging = if (@hasDecl(build_options, "verbose_logging")) + build_options.verbose_logging +else + false; + +const level: std.log.Level = if (@hasDecl(build_options, "log_level")) + std.enums.nameCast(std.log.Level, build_options.log_level) +else + .warn; + +fn log( + comptime message_level: std.log.Level, + comptime format: []const u8, + args: anytype, +) void { + if (comptime !logEnabled(message_level)) return; + + const actual_fmt = "thread {d}: " ++ format; + std.options.logFn(message_level, .zimalloc, actual_fmt, .{std.Thread.getCurrentId()} ++ args); +} + +fn logEnabled(comptime message_level: std.log.Level) bool { + return @intFromEnum(message_level) <= @intFromEnum(level); +} + +const std = @import("std"); + +const build_options = @import("build_options"); diff --git a/src/log.zig b/src/log_setCold.zig similarity index 100% rename from src/log.zig rename to src/log_setCold.zig