Skip to content
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

rework build system memory allocations #20603

Open
andrewrk opened this issue Jul 13, 2024 · 0 comments
Open

rework build system memory allocations #20603

andrewrk opened this issue Jul 13, 2024 · 0 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

Extracted from #20580.

Zig version: 0.14.0-dev.283+1d20ff11d

Currently, the build runner sets up a thread-safe arena backed by page allocation and then uses it for everything. This made sense when the build runner was a short-lived process, but now that the --watch feature is introduced, there is a cyclical component that needs to manage resources more sustainably. As it stands, a little bit of memory leaks with every detected file system update.

The goal is to move to the following scheme:

  • Use a single-threaded arena backed by page allocation for the configure phase.
  • After the configure phase ends, create a thread-safe general purpose allocator to be used by make() logic.
  • Inside make() implementations, they can create temporary arenas for allocations that need not outlive make(), or that live only until the next make() call. This decision is up to each Step implementation.
  • Watch logic needs a single-threaded general purpose allocator for its state. Or it could just use the same thread-safe one used by the make() logic.

b.allocator should be removed in order to audit all allocations. This will cause a lot of breakage in user scripts, unfortunately, but it needs to be done so that it can be replaced with b.graph.arena for the single-threaded configure-phase allocations, or a gpa parameter passed to make() - and in the latter case it needs to have memory management logic added.

After completing the rework, let's use leak detection to make sure there are no more obvious leaks for common --watch workflows. The typical workflow to quit --watch is Ctrl+C, so some debugging trick will be needed to trigger gpa.deinit().

Related:

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Jul 13, 2024
@andrewrk andrewrk added this to the 0.14.0 milestone Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. bug Observed behavior contradicts documented or intended behavior enhancement Solving this issue will likely involve adding new logic or components to the codebase. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
None yet
Development

No branches or pull requests

1 participant