Commit 274555be21ea756d8480a586f771274a79a58d80 | zig

author	Andrew Kelley <andrew@ziglang.org> 2021-12-06 16:38:12-07:00
committer	Andrew Kelley <andrew@ziglang.org> 2021-12-06 23:30:18-08:00
log	274555be21ea756d8480a586f771274a79a58d80
tree	fccbc4b05398e9ea5d29fe1ac88f0b49a0779689
parent	a711fcbbfa41d3f1dedc7ce4be9106e21b6c7ffb

stage2: improve handling of the generated file builtin.zig

All Zig code is eligible to `@import("builtin")` which is mapped to a
generated file, build.zig, based on the target and other settings.

Zig invocations which share the same target settings will generate the
same builtin.zig file and thus the path to builtin.zig is in a shared
cache folder, and different projects can sometimes use the same file.

Before this commit, this led to race conditions where multiple
invocations of `zig` would race to write this file. If one process
wanted to *read* the file while the other process *wrote* the file, the
reading process could observe a truncated or partially written
builtin.zig file.

This commit makes the following improvements:
 - limitations:
   - avoid clobbering the inode, mtime in the hot path
   - avoid creating a partially written file
   - builtin.zig needs to be on disk for debug info / stack trace purposes
   - don't mark the task as complete until the file is finished being populated
     (possibly by an external process)
 - strategy:
   - create the `@import("builtin")` `Module.File` during the AstGen
     work, based on generating the contents in memory rather than
     loading from disk.
   - write builtin.zig in a separate task that doesn't have
     to complete until the end of the AstGen work queue so that it
     can be done in parallel with everything else.
   - when writing the file, first stat the file path. If it exists, we are done.
   - otherwise, write the file to a temp file in the same directory and atomically
     rename it into place (clobbering the inode, mtime in the cold path).
 - summary:
   - all limitations respected
   - hot path: one stat() syscall that happens in a worker thread

This required adding a missing function to the standard library:
`std.fs.Dir.statFile`. In this commit, it does open() and then fstat()
which is two syscalls. It should be improved in a future commit to only
make one.

Fixes #9439.

3 files changed, 124 insertions(+), 27 deletions(-)

lib
- std
  - fs.zig
src
- Compilation.zig
- Module.zig

lib/std/fs.zig+10

...	@@ -2170,6 +2170,16 @@ pub const Dir = struct {	...	@@ -2170,6 +2170,16 @@ pub const Dir = struct {
2170	return file.stat();	2170	return file.stat();
2171	}	2171	}
2172		2172
		2173	pub const StatFileError = File.OpenError \|\| StatError;
		2174
		2175	// TODO: improve this to use the fstatat syscall instead of making 2 syscalls here.
		2176	pub fn statFile(self: Dir, sub_path: []const u8) StatFileError!File.Stat {
		2177	var file = try self.openFile(sub_path, .{});
		2178	defer file.close();
		2179
		2180	return file.stat();
		2181	}
		2182
2173	pub const ChmodError = File.ChmodError;	2183	pub const ChmodError = File.ChmodError;
2174		2184
2175	/// Changes the mode of the directory.	2185	/// Changes the mode of the directory.

src/Compilation.zig+42-27

 @@ -2152,15 +2152,6 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
     defer main_progress_node.end();
     if (self.color == .off) progress.terminal = null;
-    // If we need to write out builtin.zig, it needs to be done before starting
-    // the AstGen tasks.
-    if (self.bin_file.options.module) |mod| {
-        if (mod.job_queued_update_builtin_zig) {
-            mod.job_queued_update_builtin_zig = false;
-            try self.updateBuiltinZigFile(mod);
-        }
-    }
     // Here we queue up all the AstGen tasks first, followed by C object compilation.
     // We wait until the AstGen tasks are all completed before proceeding to the
     // (at least for now) single-threaded main work queue. However, C object compilation
 @@ -2185,6 +2176,21 @@ pub fn performAllTheWork(self: *Compilation) error{ TimerUnsupported, OutOfMemor
         self.astgen_wait_group.reset();
         defer self.astgen_wait_group.wait();
+        // builtin.zig is handled specially for two reasons:
+        // 1. to avoid race condition of zig processes truncating each other's builtin.zig files
+        // 2. optimization; in the hot path it only incurs a stat() syscall, which happens
+        //    in the `astgen_wait_group`.
+        if (self.bin_file.options.module) |mod| {
+            if (mod.job_queued_update_builtin_zig) {
+                mod.job_queued_update_builtin_zig = false;
+                self.astgen_wait_group.start();
+                try self.thread_pool.spawn(workerUpdateBuiltinZigFile, .{
+                    self, mod, &self.astgen_wait_group,
+                });
+            }
+        }
         while (self.astgen_work_queue.readItem()) |file| {
             self.astgen_wait_group.start();
             try self.thread_pool.spawn(workerAstGenFile, .{
 @@ -2748,6 +2754,8 @@ fn workerAstGenFile(
             extra_index = item.end;
             const import_path = file.zir.nullTerminatedString(item.data.name);
+            // `@import("builtin")` is handled specially.
+            if (mem.eql(u8, import_path, "builtin")) continue;
             const import_result = blk: {
                 comp.mutex.lock();
 @@ -2775,6 +2783,29 @@ fn workerAstGenFile(
     }
 }
+fn workerUpdateBuiltinZigFile(
+    comp: *Compilation,
+    mod: *Module,
+    wg: *WaitGroup,
+) void {
+    defer wg.finish();
+    mod.populateBuiltinFile() catch |err| {
+        const dir_path: []const u8 = mod.zig_cache_artifact_directory.path orelse ".";
+        comp.mutex.lock();
+        defer comp.mutex.unlock();
+        comp.setMiscFailure(.write_builtin_zig, "unable to write builtin.zig to {s}: {s}", .{
+            dir_path, @errorName(err),
+        }) catch |oom| switch (oom) {
+            error.OutOfMemory => log.err("unable to write builtin.zig to {s}: {s}", .{
+                dir_path, @errorName(err),
+            }),
+        };
+    };
+}
 fn workerCheckEmbedFile(
     comp: *Compilation,
     embed_file: *Module.EmbedFile,
 @@ -4046,22 +4077,6 @@ fn wantBuildLibUnwindFromSource(comp: *Compilation) bool {
         comp.bin_file.options.object_format != .c;
 }
-fn updateBuiltinZigFile(comp: *Compilation, mod: *Module) Allocator.Error!void {
-    const tracy_trace = trace(@src());
-    defer tracy_trace.end();
-    const source = try comp.generateBuiltinZigSource(comp.gpa);
-    defer comp.gpa.free(source);
-    mod.zig_cache_artifact_directory.handle.writeFile("builtin.zig", source) catch |err| {
-        const dir_path: []const u8 = mod.zig_cache_artifact_directory.path orelse ".";
-        try comp.setMiscFailure(.write_builtin_zig, "unable to write builtin.zig to {s}: {s}", .{
-            dir_path,
-            @errorName(err),
-        });
-    };
-}
 fn setMiscFailure(
     comp: *Compilation,
     tag: MiscTask,
 @@ -4084,7 +4099,7 @@ pub fn dump_argv(argv: []const []const u8) void {
     std.debug.print("{s}\n", .{argv[argv.len - 1]});
 }
-pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Allocator.Error![]u8 {
+pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Allocator.Error![:0]u8 {
     const tracy_trace = trace(@src());
     defer tracy_trace.end();
 @@ -4290,7 +4305,7 @@ pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Alloca
         }
     }
-    return buffer.toOwnedSlice();
+    return buffer.toOwnedSliceSentinel(0);
 }
 pub fn updateSubCompilation(sub_compilation: *Compilation) !void {

src/Module.zig+72

...	@@ -2967,6 +2967,78 @@ fn updateZirRefs(gpa: Allocator, file: *File, old_zir: Zir) !void {	...	@@ -2967,6 +2967,78 @@ fn updateZirRefs(gpa: Allocator, file: *File, old_zir: Zir) !void {
2967	}	2967	}
2968	}	2968	}
2969		2969
		2970	pub fn populateBuiltinFile(mod: *Module) !void {
		2971	const tracy = trace(@src());
		2972	defer tracy.end();
		2973
		2974	const comp = mod.comp;
		2975	const pkg_and_file = blk: {
		2976	comp.mutex.lock();
		2977	defer comp.mutex.unlock();
		2978
		2979	const builtin_pkg = mod.main_pkg.table.get("builtin").?;
		2980	const result = try mod.importPkg(builtin_pkg);
		2981	break :blk .{
		2982	.file = result.file,
		2983	.pkg = builtin_pkg,
		2984	};
		2985	};
		2986	const file = pkg_and_file.file;
		2987	const builtin_pkg = pkg_and_file.pkg;
		2988	const gpa = mod.gpa;
		2989	file.source = try comp.generateBuiltinZigSource(gpa);
		2990	file.source_loaded = true;
		2991
		2992	if (builtin_pkg.root_src_directory.handle.statFile(builtin_pkg.root_src_path)) \|stat\| {
		2993	if (stat.size != file.source.len) {
		2994	const full_path = try builtin_pkg.root_src_directory.join(gpa, &.{
		2995	builtin_pkg.root_src_path,
		2996	});
		2997	defer gpa.free(full_path);
		2998
		2999	log.warn(
		3000	"the cached file '{s}' had the wrong size. Expected {d}, found {d}. " ++
		3001	"Overwriting with correct file contents now",
		3002	.{ full_path, file.source.len, stat.size },
		3003	);
		3004
		3005	try writeBuiltinFile(file, builtin_pkg);
		3006	} else {
		3007	file.stat_size = stat.size;
		3008	file.stat_inode = stat.inode;
		3009	file.stat_mtime = stat.mtime;
		3010	}
		3011	} else \|err\| switch (err) {
		3012	error.BadPathName => unreachable, // it's always "builtin.zig"
		3013	error.NameTooLong => unreachable, // it's always "builtin.zig"
		3014	error.PipeBusy => unreachable, // it's not a pipe
		3015	error.WouldBlock => unreachable, // not asking for non-blocking I/O
		3016
		3017	error.FileNotFound => try writeBuiltinFile(file, builtin_pkg),
		3018
		3019	else => \|e\| return e,
		3020	}
		3021
		3022	file.tree = try std.zig.parse(gpa, file.source);
		3023	file.tree_loaded = true;
		3024	assert(file.tree.errors.len == 0); // builtin.zig must parse
		3025
		3026	file.zir = try AstGen.generate(gpa, file.tree);
		3027	file.zir_loaded = true;
		3028	file.status = .success_zir;
		3029	}
		3030
		3031	pub fn writeBuiltinFile(file: File, builtin_pkg: Package) !void {
		3032	var af = try builtin_pkg.root_src_directory.handle.atomicFile(builtin_pkg.root_src_path, .{});
		3033	defer af.deinit();
		3034	try af.file.writeAll(file.source);
		3035	try af.finish();
		3036
		3037	file.stat_size = file.source.len;
		3038	file.stat_inode = 0; // dummy value
		3039	file.stat_mtime = 0; // dummy value
		3040	}
		3041
2970	pub fn mapOldZirToNew(	3042	pub fn mapOldZirToNew(
2971	gpa: Allocator,	3043	gpa: Allocator,
2972	old_zir: Zir,	3044	old_zir: Zir,