It’s not quite clear to me what’s the intended allocation pattern when writing build.zig. Let’s say I am writing an
pub fn build(b: *std.build.Builder) void {
// my code here
}
Is the following correct:
I can use b.allocator as my general purpose allocator, I don’t need to create my own var gpa = std.heap.GeneralPurposeAllocator(.{}){};
The scope of b.allocator covers both construction of build graph (the fn build) as well as the execution of actual steps. That is, that steps can “close over” b.allocator and use it at runtime, it is not an arena scoped purely to the build
In general, I don’t have to worry about freeing things. Steps don’t have deinit function, and that’s ok and by design.
Looking at the implementation:
pub fn main() !void {
// Here we use an ArenaAllocator backed by a DirectAllocator because a build is a short-lived,
// one shot program. We don't need to waste time freeing memory and finding places to squish
// bytes into. So we free everything all at once at the very end.
var single_threaded_arena = std.heap.ArenaAllocator.init(std.heap.page_allocator);
defer single_threaded_arena.deinit();
var thread_safe_arena: std.heap.ThreadSafeAllocator = .{
.child_allocator = single_threaded_arena.allocator(),
};
const arena = thread_safe_arena.allocator();
It looks the answer is “yes, on all three counts” at least for what’s currently implemented.
And I guess the fourth “yes” question: Is a.create(T) catch @panic("OOM") the correct way to handle allocation failure in build.zig?
The relevant issue is improve std.build to set a good example of proper error handling on out of memory · Issue #8350 · ziglang/zig · GitHub which has been closed with catch @panic("OOM"); seeming to be the preferred way to handle allocation errors in build.zig.
2 Likes
Looked again at this today, and noticed that the Build struct is itself confused about memory management (or I am misreading the code).
There’s a destroy method, so it sorta tries to cleanup after itself:
But then there’s a bunch of data it forgets to cleanup:
.user_input_options = UserInputOptionsMap.init(allocator),
.available_options_map = AvailableOptionsMap.init(allocator),
.available_options_list = ArrayList(AvailableOption).init(allocator),
And we are not handling allocation failure here:
try self.top_level_steps.put(allocator, self.install_tls.step.name, &self.install_tls);
try self.top_level_steps.put(allocator, self.uninstall_tls.step.name, &self.uninstall_tls);
I wonder if we should just go and:
Rename Build.allocator to Build.arena
Clearly document that the arena is permanent
Remove destroy method.
2 Likes
matklad:
Rename Build.allocator to Build.arena
Clearly document that the arena is permanent
Remove destroy method.
Agreed.
Here are a couple relevant issues:
opened 10:33PM - 16 Mar 23 UTC
breaking
zig build system
Extracted from #14647.
I've seen quite a bit of build.zig code in the wild th… at does inappropriate things, such as call make() during build(), or create steps during make(). This issue is to set a global enum state to which phase is currently happening, and enforce that the API is only used in the intended phase.
opened 12:06AM - 13 Jan 23 UTC
enhancement
breaking
frontend
zig build system
Extracted from #14265.
build.zig is handy for adding build logic, but it's no… t handy to trust a lot of different people's code running natively on the host. A deep dependency tree has the problem that many different build.zig scripts are all running, and the chance of insufficient auditing grows quickly the more dependencies there are.
This issue is to make the zig build system compile build.zig to wasm32-wasi instead of natively, and merely output its build graph, based on the user-provided build options, to stdout. At this point a separate build_runner can execute the requested build steps (or not, depending on the permissions granted).
Perhaps a middle ground, here, would be to run the make() steps directly in the WASI code. But eventually the idea would be that make() steps happen by a separate build runner, not by the wasm guest.
I can foresee a potential escape hatch for opting into running build.zig directly on the host. As an example, the Android SDK package wants to access the Windows registry in some cases. However, I would like to avoid even having this escape hatch if possible.
This issue will require Zig to gain a WASI interpreter, written in Zig. Whereever will we find one?
These two phases will see some separation introduced, including potentially being executed in different processes or contexts. Point being it’s fine to use the same allocator in both, but you should not rely on the allocator being the same instance. I think it’s unlikely this constraint will ever be a problem.
2 Likes