From abf895595189eb45df8c97f4029c58976815b450 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 14 Jul 2024 19:48:08 -0700 Subject: make zig compiler processes live across rebuilds Changes the `make` function signature to take an options struct, which additionally includes `watch: bool`. I intentionally am not exposing this information to configure phase logic. Also adds global zig cache to the compiler cache prefixes. Closes #20600 --- lib/std/Build/Step.zig | 157 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 112 insertions(+), 45 deletions(-) (limited to 'lib/std/Build/Step.zig') diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index 3c6cd660ff..397ad6c55f 100644 --- a/lib/std/Build/Step.zig +++ b/lib/std/Build/Step.zig @@ -68,7 +68,13 @@ pub const TestResults = struct { } }; -pub const MakeFn = *const fn (step: *Step, prog_node: std.Progress.Node) anyerror!void; +pub const MakeOptions = struct { + progress_node: std.Progress.Node, + thread_pool: *std.Thread.Pool, + watch: bool, +}; + +pub const MakeFn = *const fn (step: *Step, options: MakeOptions) anyerror!void; pub const State = enum { precheck_unstarted, @@ -219,10 +225,10 @@ pub fn init(options: StepOptions) Step { /// If the Step's `make` function reports `error.MakeFailed`, it indicates they /// have already reported the error. Otherwise, we add a simple error report /// here. -pub fn make(s: *Step, prog_node: std.Progress.Node) error{ MakeFailed, MakeSkipped }!void { +pub fn make(s: *Step, options: MakeOptions) error{ MakeFailed, MakeSkipped }!void { const arena = s.owner.allocator; - s.makeFn(s, prog_node) catch |err| switch (err) { + s.makeFn(s, options) catch |err| switch (err) { error.MakeFailed => return error.MakeFailed, error.MakeSkipped => return error.MakeSkipped, else => { @@ -260,8 +266,8 @@ pub fn getStackTrace(s: *Step) ?std.builtin.StackTrace { }; } -fn makeNoOp(step: *Step, prog_node: std.Progress.Node) anyerror!void { - _ = prog_node; +fn makeNoOp(step: *Step, options: MakeOptions) anyerror!void { + _ = options; var all_cached = true; @@ -352,13 +358,25 @@ pub fn addError(step: *Step, comptime fmt: []const u8, args: anytype) error{OutO try step.result_error_msgs.append(arena, msg); } +pub const ZigProcess = struct { + child: std.process.Child, + poller: std.io.Poller(StreamEnum), + + pub const StreamEnum = enum { stdout, stderr }; +}; + /// Assumes that argv contains `--listen=-` and that the process being spawned /// is the zig compiler - the same version that compiled the build runner. pub fn evalZigProcess( s: *Step, argv: []const []const u8, prog_node: std.Progress.Node, + watch: bool, ) !?[]const u8 { + if (s.getZigProcess()) |zp| { + assert(watch); + return zigProcessUpdate(s, zp, watch); + } assert(argv.len != 0); const b = s.owner; const arena = b.allocator; @@ -378,29 +396,76 @@ pub fn evalZigProcess( child.spawn() catch |err| return s.fail("unable to spawn {s}: {s}", .{ argv[0], @errorName(err), }); - var timer = try std.time.Timer.start(); - var poller = std.io.poll(gpa, enum { stdout, stderr }, .{ - .stdout = child.stdout.?, - .stderr = child.stderr.?, - }); - defer poller.deinit(); + const zp = try arena.create(ZigProcess); + zp.* = .{ + .child = child, + .poller = std.io.poll(gpa, ZigProcess.StreamEnum, .{ + .stdout = child.stdout.?, + .stderr = child.stderr.?, + }), + }; + if (watch) s.setZigProcess(zp); + defer if (!watch) zp.poller.deinit(); + + const result = try zigProcessUpdate(s, zp, watch); + + if (!watch) { + // Send EOF to stdin. + zp.child.stdin.?.close(); + zp.child.stdin = null; + + const term = zp.child.wait() catch |err| { + return s.fail("unable to wait for {s}: {s}", .{ argv[0], @errorName(err) }); + }; + s.result_peak_rss = zp.child.resource_usage_statistics.getMaxRss() orelse 0; + + // Special handling for Compile step that is expecting compile errors. + if (s.cast(Compile)) |compile| switch (term) { + .Exited => { + // Note that the exit code may be 0 in this case due to the + // compiler server protocol. + if (compile.expect_errors != null) { + return error.NeedCompileErrorCheck; + } + }, + else => {}, + }; + + try handleChildProcessTerm(s, term, null, argv); + } + + if (s.result_error_bundle.errorMessageCount() > 0) { + return s.fail("the following command failed with {d} compilation errors:\n{s}", .{ + s.result_error_bundle.errorMessageCount(), + try allocPrintCmd(arena, null, argv), + }); + } + + return result; +} - try sendMessage(child.stdin.?, .update); - try sendMessage(child.stdin.?, .exit); +fn zigProcessUpdate(s: *Step, zp: *ZigProcess, watch: bool) !?[]const u8 { + const b = s.owner; + const arena = b.allocator; + + var timer = try std.time.Timer.start(); + + try sendMessage(zp.child.stdin.?, .update); + if (!watch) try sendMessage(zp.child.stdin.?, .exit); const Header = std.zig.Server.Message.Header; var result: ?[]const u8 = null; - const stdout = poller.fifo(.stdout); + const stdout = zp.poller.fifo(.stdout); poll: while (true) { while (stdout.readableLength() < @sizeOf(Header)) { - if (!(try poller.poll())) break :poll; + if (!(try zp.poller.poll())) break :poll; } const header = stdout.reader().readStruct(Header) catch unreachable; while (stdout.readableLength() < header.bytes_len) { - if (!(try poller.poll())) break :poll; + if (!(try zp.poller.poll())) break :poll; } const body = stdout.readableSliceOfLen(header.bytes_len); @@ -428,12 +493,22 @@ pub fn evalZigProcess( .string_bytes = try arena.dupe(u8, string_bytes), .extra = extra_array, }; + if (watch) { + // This message indicates the end of the update. + stdout.discard(body.len); + break; + } }, .emit_bin_path => { const EbpHdr = std.zig.Server.Message.EmitBinPath; const ebp_hdr = @as(*align(1) const EbpHdr, @ptrCast(body)); s.result_cached = ebp_hdr.flags.cache_hit; result = try arena.dupe(u8, body[@sizeOf(EbpHdr)..]); + if (watch) { + // This message indicates the end of the update. + stdout.discard(body.len); + break; + } }, .file_system_inputs => { s.clearWatchInputs(); @@ -470,6 +545,13 @@ pub fn evalZigProcess( }; try addWatchInputFromPath(s, path, std.fs.path.basename(sub_path)); }, + .global_cache => { + const path: Build.Cache.Path = .{ + .root_dir = s.owner.graph.global_cache_root, + .sub_path = sub_path_dirname, + }; + try addWatchInputFromPath(s, path, std.fs.path.basename(sub_path)); + }, } } }, @@ -479,43 +561,28 @@ pub fn evalZigProcess( stdout.discard(body.len); } - const stderr = poller.fifo(.stderr); + s.result_duration_ns = timer.read(); + + const stderr = zp.poller.fifo(.stderr); if (stderr.readableLength() > 0) { try s.result_error_msgs.append(arena, try stderr.toOwnedSlice()); } - // Send EOF to stdin. - child.stdin.?.close(); - child.stdin = null; + return result; +} - const term = child.wait() catch |err| { - return s.fail("unable to wait for {s}: {s}", .{ argv[0], @errorName(err) }); +fn getZigProcess(s: *Step) ?*ZigProcess { + return switch (s.id) { + .compile => s.cast(Compile).?.zig_process, + else => null, }; - s.result_duration_ns = timer.read(); - s.result_peak_rss = child.resource_usage_statistics.getMaxRss() orelse 0; - - // Special handling for Compile step that is expecting compile errors. - if (s.cast(Compile)) |compile| switch (term) { - .Exited => { - // Note that the exit code may be 0 in this case due to the - // compiler server protocol. - if (compile.expect_errors != null) { - return error.NeedCompileErrorCheck; - } - }, - else => {}, - }; - - try handleChildProcessTerm(s, term, null, argv); +} - if (s.result_error_bundle.errorMessageCount() > 0) { - return s.fail("the following command failed with {d} compilation errors:\n{s}", .{ - s.result_error_bundle.errorMessageCount(), - try allocPrintCmd(arena, null, argv), - }); +fn setZigProcess(s: *Step, zp: *ZigProcess) void { + switch (s.id) { + .compile => s.cast(Compile).?.zig_process = zp, + else => unreachable, } - - return result; } fn sendMessage(file: std.fs.File, tag: std.zig.Client.Message.Tag) !void { -- cgit v1.2.3 From f6c1b71c220406d60ca4bdf9c949e775f7fca466 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 14 Jul 2024 21:38:55 -0700 Subject: build system: update std.Progress.Node for long-lived children --- lib/std/Build/Step.zig | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'lib/std/Build/Step.zig') diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index 397ad6c55f..fc40a680c4 100644 --- a/lib/std/Build/Step.zig +++ b/lib/std/Build/Step.zig @@ -361,6 +361,7 @@ pub fn addError(step: *Step, comptime fmt: []const u8, args: anytype) error{OutO pub const ZigProcess = struct { child: std.process.Child, poller: std.io.Poller(StreamEnum), + progress_ipc_fd: if (std.Progress.have_ipc) ?std.posix.fd_t else void, pub const StreamEnum = enum { stdout, stderr }; }; @@ -375,6 +376,7 @@ pub fn evalZigProcess( ) !?[]const u8 { if (s.getZigProcess()) |zp| { assert(watch); + if (std.Progress.have_ipc) if (zp.progress_ipc_fd) |fd| prog_node.setIpcFd(fd); return zigProcessUpdate(s, zp, watch); } assert(argv.len != 0); @@ -404,6 +406,7 @@ pub fn evalZigProcess( .stdout = child.stdout.?, .stderr = child.stderr.?, }), + .progress_ipc_fd = if (std.Progress.have_ipc) child.progress_node.getIpcFd() else {}, }; if (watch) s.setZigProcess(zp); defer if (!watch) zp.poller.deinit(); @@ -435,6 +438,8 @@ pub fn evalZigProcess( try handleChildProcessTerm(s, term, null, argv); } + // This is intentionally printed for failure on the first build but not for + // subsequent rebuilds. if (s.result_error_bundle.errorMessageCount() > 0) { return s.fail("the following command failed with {d} compilation errors:\n{s}", .{ s.result_error_bundle.errorMessageCount(), -- cgit v1.2.3 From 987f63208e4bdd044768465f1581d03ee8964e28 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 14 Jul 2024 22:17:29 -0700 Subject: build runner: handle compiler subprocess failures gracefully Compilation errors now report a failure on rebuilds triggered by file system watches. Compiler crashes now report failure correctly on rebuilds triggered by file system watches. The compiler subprocess is restarted if a broken pipe is encountered on a rebuild. --- lib/std/Build/Step.zig | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) (limited to 'lib/std/Build/Step.zig') diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index fc40a680c4..661ee58509 100644 --- a/lib/std/Build/Step.zig +++ b/lib/std/Build/Step.zig @@ -374,10 +374,37 @@ pub fn evalZigProcess( prog_node: std.Progress.Node, watch: bool, ) !?[]const u8 { - if (s.getZigProcess()) |zp| { + if (s.getZigProcess()) |zp| update: { assert(watch); if (std.Progress.have_ipc) if (zp.progress_ipc_fd) |fd| prog_node.setIpcFd(fd); - return zigProcessUpdate(s, zp, watch); + const result = zigProcessUpdate(s, zp, watch) catch |err| switch (err) { + error.BrokenPipe => { + // Process restart required. + const term = zp.child.wait() catch |e| { + return s.fail("unable to wait for {s}: {s}", .{ argv[0], @errorName(e) }); + }; + _ = term; + s.clearZigProcess(); + break :update; + }, + else => |e| return e, + }; + + if (s.result_error_bundle.errorMessageCount() > 0) + return s.fail("{d} compilation errors", .{s.result_error_bundle.errorMessageCount()}); + + if (s.result_error_msgs.items.len > 0 and result == null) { + // Crash detected. + const term = zp.child.wait() catch |e| { + return s.fail("unable to wait for {s}: {s}", .{ argv[0], @errorName(e) }); + }; + s.result_peak_rss = zp.child.resource_usage_statistics.getMaxRss() orelse 0; + s.clearZigProcess(); + try handleChildProcessTerm(s, term, null, argv); + return error.MakeFailed; + } + + return result; } assert(argv.len != 0); const b = s.owner; @@ -399,7 +426,7 @@ pub fn evalZigProcess( argv[0], @errorName(err), }); - const zp = try arena.create(ZigProcess); + const zp = try gpa.create(ZigProcess); zp.* = .{ .child = child, .poller = std.io.poll(gpa, ZigProcess.StreamEnum, .{ @@ -590,6 +617,20 @@ fn setZigProcess(s: *Step, zp: *ZigProcess) void { } } +fn clearZigProcess(s: *Step) void { + const gpa = s.owner.allocator; + switch (s.id) { + .compile => { + const compile = s.cast(Compile).?; + if (compile.zig_process) |zp| { + gpa.destroy(zp); + compile.zig_process = null; + } + }, + else => unreachable, + } +} + fn sendMessage(file: std.fs.File, tag: std.zig.Client.Message.Tag) !void { const header: std.zig.Client.Message.Header = .{ .tag = tag, -- cgit v1.2.3 From 445bd7a06fc34c9a59c6458774769bfaa2757a2f Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 14 Jul 2024 22:27:51 -0700 Subject: build runner: update watch caption to include subprocesses --- lib/compiler/build_runner.zig | 13 ++++++++++--- lib/std/Build.zig | 2 +- lib/std/Build/Step.zig | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) (limited to 'lib/std/Build/Step.zig') diff --git a/lib/compiler/build_runner.zig b/lib/compiler/build_runner.zig index af65d10948..1b13881586 100644 --- a/lib/compiler/build_runner.zig +++ b/lib/compiler/build_runner.zig @@ -72,7 +72,6 @@ pub fn main() !void { .query = .{}, .result = try std.zig.system.resolveTargetQuery(.{}), }, - .incremental = null, }; graph.cache.addPrefix(.{ .path = null, .handle = std.fs.cwd() }); @@ -411,8 +410,8 @@ pub fn main() !void { // trigger a rebuild on all steps with modified inputs, as well as their // recursive dependants. var caption_buf: [std.Progress.Node.max_name_len]u8 = undefined; - const caption = std.fmt.bufPrint(&caption_buf, "Watching {d} Directories", .{ - w.dir_table.entries.len, + const caption = std.fmt.bufPrint(&caption_buf, "watching {d} directories, {d} processes", .{ + w.dir_table.entries.len, countSubProcesses(run.step_stack.keys()), }) catch &caption_buf; var debouncing_node = main_progress_node.start(caption, 0); var debounce_timeout: Watch.Timeout = .none; @@ -445,6 +444,14 @@ fn markFailedStepsDirty(gpa: Allocator, all_steps: []const *Step) void { }; } +fn countSubProcesses(all_steps: []const *Step) usize { + var count: usize = 0; + for (all_steps) |s| { + count += @intFromBool(s.getZigProcess() != null); + } + return count; +} + const Run = struct { max_rss: u64, max_rss_is_default: bool, diff --git a/lib/std/Build.zig b/lib/std/Build.zig index 29621f95c9..06de7aa6f4 100644 --- a/lib/std/Build.zig +++ b/lib/std/Build.zig @@ -120,7 +120,7 @@ pub const Graph = struct { needed_lazy_dependencies: std.StringArrayHashMapUnmanaged(void) = .{}, /// Information about the native target. Computed before build() is invoked. host: ResolvedTarget, - incremental: ?bool, + incremental: ?bool = null, }; const AvailableDeps = []const struct { []const u8, []const u8 }; diff --git a/lib/std/Build/Step.zig b/lib/std/Build/Step.zig index 661ee58509..8f3236d867 100644 --- a/lib/std/Build/Step.zig +++ b/lib/std/Build/Step.zig @@ -603,7 +603,7 @@ fn zigProcessUpdate(s: *Step, zp: *ZigProcess, watch: bool) !?[]const u8 { return result; } -fn getZigProcess(s: *Step) ?*ZigProcess { +pub fn getZigProcess(s: *Step) ?*ZigProcess { return switch (s.id) { .compile => s.cast(Compile).?.zig_process, else => null, -- cgit v1.2.3