authorgravatar for jacobly@ziglang.orgJacob Young <jacobly@ziglang.org> 2023-03-06 17:22:23-05:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2023-03-08 00:00:52-05:00
loge3cf9d165081533524927cebf8a92ac6fee097f2
treefa2c60e017e3fb75019d53e579dfe2c8c2e15998
parent6fc1621cbd33f388388844bfc176a10c062779cc

Module: rewrite zir caching logic

Multiple processes can sit waiting for the exclusive lock at the same time, so we want to recheck whether it needs to be updated whenever we get an exclusive lock. This also fixes a race condition between one process truncating the cache file and another process reading it without atomic locking.

2 files changed, 62 insertions(+), 67 deletions(-)

src/Module.zig+62-65
...@@ -3538,44 +3538,61 @@ pub fn astGenFile(mod: *Module, file: *File) !void {...@@ -3538,44 +3538,61 @@ pub fn astGenFile(mod: *Module, file: *File) !void {
3538 const cache_directory = if (want_local_cache) mod.local_zir_cache else mod.global_zir_cache;3538 const cache_directory = if (want_local_cache) mod.local_zir_cache else mod.global_zir_cache;
3539 const zir_dir = cache_directory.handle;3539 const zir_dir = cache_directory.handle;
35403540
3541 var cache_file: ?std.fs.File = null;
3542 defer if (cache_file) |f| f.close();
3543
3544 // Determine whether we need to reload the file from disk and redo parsing and AstGen.3541 // Determine whether we need to reload the file from disk and redo parsing and AstGen.
3545 switch (file.status) {3542 var lock: std.fs.File.Lock = switch (file.status) {
3546 .never_loaded, .retryable_failure => cached: {3543 .never_loaded, .retryable_failure => lock: {
3547 // First, load the cached ZIR code, if any.3544 // First, load the cached ZIR code, if any.
3548 log.debug("AstGen checking cache: {s} (local={}, digest={s})", .{3545 log.debug("AstGen checking cache: {s} (local={}, digest={s})", .{
3549 file.sub_file_path, want_local_cache, &digest,3546 file.sub_file_path, want_local_cache, &digest,
3550 });3547 });
35513548
3552 // We ask for a lock in order to coordinate with other zig processes.3549 break :lock .Shared;
3553 // If another process is already working on this file, we will get the cached3550 },
3554 // version. Likewise if we're working on AstGen and another process asks for3551 .parse_failure, .astgen_failure, .success_zir => lock: {
3555 // the cached file, they'll get it.3552 const unchanged_metadata =
3556 cache_file = zir_dir.openFile(&digest, .{ .lock = .Shared }) catch |err| switch (err) {3553 stat.size == file.stat.size and
3557 error.PathAlreadyExists => unreachable, // opening for reading3554 stat.mtime == file.stat.mtime and
3558 error.NoSpaceLeft => unreachable, // opening for reading3555 stat.inode == file.stat.inode;
3559 error.NotDir => unreachable, // no dir components3556
3560 error.InvalidUtf8 => unreachable, // it's a hex encoded name3557 if (unchanged_metadata) {
3561 error.BadPathName => unreachable, // it's a hex encoded name3558 log.debug("unmodified metadata of file: {s}", .{file.sub_file_path});
3562 error.NameTooLong => unreachable, // it's a fixed size name3559 return;
3563 error.PipeBusy => unreachable, // it's not a pipe3560 }
3564 error.WouldBlock => unreachable, // not asking for non-blocking I/O3561
35653562 log.debug("metadata changed: {s}", .{file.sub_file_path});
3566 error.SymLinkLoop,3563
3567 error.FileNotFound,3564 break :lock .Exclusive;
3568 error.Unexpected,3565 },
3569 => break :cached,3566 };
35703567
3571 else => |e| return e, // Retryable errors are handled at callsite.3568 // We ask for a lock in order to coordinate with other zig processes.
3572 };3569 // If another process is already working on this file, we will get the cached
3570 // version. Likewise if we're working on AstGen and another process asks for
3571 // the cached file, they'll get it.
3572 const cache_file = zir_dir.createFile(&digest, .{
3573 .read = true,
3574 .truncate = false,
3575 .lock = lock,
3576 }) catch |err| switch (err) {
3577 error.NotDir => unreachable, // no dir components
3578 error.InvalidUtf8 => unreachable, // it's a hex encoded name
3579 error.BadPathName => unreachable, // it's a hex encoded name
3580 error.NameTooLong => unreachable, // it's a fixed size name
3581 error.PipeBusy => unreachable, // it's not a pipe
3582 error.WouldBlock => unreachable, // not asking for non-blocking I/O
3583 error.FileNotFound => unreachable, // no dir components
3584
3585 else => |e| return e, // Retryable errors are handled at callsite.
3586 };
3587 defer cache_file.close();
35733588
3589 while (true) {
3590 update: {
3574 // First we read the header to determine the lengths of arrays.3591 // First we read the header to determine the lengths of arrays.
3575 const header = cache_file.?.reader().readStruct(Zir.Header) catch |err| switch (err) {3592 const header = cache_file.reader().readStruct(Zir.Header) catch |err| switch (err) {
3576 // This can happen if Zig bails out of this function between creating3593 // This can happen if Zig bails out of this function between creating
3577 // the cached file and writing it.3594 // the cached file and writing it.
3578 error.EndOfStream => break :cached,3595 error.EndOfStream => break :update,
3579 else => |e| return e,3596 else => |e| return e,
3580 };3597 };
3581 const unchanged_metadata =3598 const unchanged_metadata =
...@@ -3585,7 +3602,7 @@ pub fn astGenFile(mod: *Module, file: *File) !void {...@@ -3585,7 +3602,7 @@ pub fn astGenFile(mod: *Module, file: *File) !void {
35853602
3586 if (!unchanged_metadata) {3603 if (!unchanged_metadata) {
3587 log.debug("AstGen cache stale: {s}", .{file.sub_file_path});3604 log.debug("AstGen cache stale: {s}", .{file.sub_file_path});
3588 break :cached;3605 break :update;
3589 }3606 }
3590 log.debug("AstGen cache hit: {s} instructions_len={d}", .{3607 log.debug("AstGen cache hit: {s} instructions_len={d}", .{
3591 file.sub_file_path, header.instructions_len,3608 file.sub_file_path, header.instructions_len,
...@@ -3637,13 +3654,13 @@ pub fn astGenFile(mod: *Module, file: *File) !void {...@@ -3637,13 +3654,13 @@ pub fn astGenFile(mod: *Module, file: *File) !void {
3637 .iov_len = header.extra_len * 4,3654 .iov_len = header.extra_len * 4,
3638 },3655 },
3639 };3656 };
3640 const amt_read = try cache_file.?.readvAll(&iovecs);3657 const amt_read = try cache_file.readvAll(&iovecs);
3641 const amt_expected = zir.instructions.len * 9 +3658 const amt_expected = zir.instructions.len * 9 +
3642 zir.string_bytes.len +3659 zir.string_bytes.len +
3643 zir.extra.len * 4;3660 zir.extra.len * 4;
3644 if (amt_read != amt_expected) {3661 if (amt_read != amt_expected) {
3645 log.warn("unexpected EOF reading cached ZIR for {s}", .{file.sub_file_path});3662 log.warn("unexpected EOF reading cached ZIR for {s}", .{file.sub_file_path});
3646 break :cached;3663 break :update;
3647 }3664 }
3648 if (data_has_safety_tag) {3665 if (data_has_safety_tag) {
3649 const tags = zir.instructions.items(.tag);3666 const tags = zir.instructions.items(.tag);
...@@ -3679,42 +3696,22 @@ pub fn astGenFile(mod: *Module, file: *File) !void {...@@ -3679,42 +3696,22 @@ pub fn astGenFile(mod: *Module, file: *File) !void {
3679 return error.AnalysisFail;3696 return error.AnalysisFail;
3680 }3697 }
3681 return;3698 return;
3682 },3699 }
3683 .parse_failure, .astgen_failure, .success_zir => {
3684 const unchanged_metadata =
3685 stat.size == file.stat.size and
3686 stat.mtime == file.stat.mtime and
3687 stat.inode == file.stat.inode;
3688
3689 if (unchanged_metadata) {
3690 log.debug("unmodified metadata of file: {s}", .{file.sub_file_path});
3691 return;
3692 }
36933700
3694 log.debug("metadata changed: {s}", .{file.sub_file_path});3701 // If we already have the exclusive lock then it is our job to update.
3695 },3702 if (builtin.os.tag == .wasi or lock == .Exclusive) break;
3696 }3703 // Otherwise, unlock to give someone a chance to get the exclusive lock
3697 if (cache_file) |f| {3704 // and then upgrade to an exclusive lock.
3698 f.close();3705 cache_file.unlock();
3699 cache_file = null;3706 lock = .Exclusive;
3707 try cache_file.lock(lock);
3700 }3708 }
3701 cache_file = zir_dir.createFile(&digest, .{ .lock = .Exclusive }) catch |err| switch (err) {
3702 error.NotDir => unreachable, // no dir components
3703 error.InvalidUtf8 => unreachable, // it's a hex encoded name
3704 error.BadPathName => unreachable, // it's a hex encoded name
3705 error.NameTooLong => unreachable, // it's a fixed size name
3706 error.PipeBusy => unreachable, // it's not a pipe
3707 error.WouldBlock => unreachable, // not asking for non-blocking I/O
3708 error.FileNotFound => unreachable, // no dir components
37093709
3710 else => |e| {3710 // The cache is definitely stale so delete the contents to avoid an underwrite later.
3711 const pkg_path = file.pkg.root_src_directory.path orelse ".";3711 cache_file.setEndPos(0) catch |err| switch (err) {
3712 const cache_path = cache_directory.path orelse ".";3712 error.FileTooBig => unreachable, // 0 is not too big
3713 log.warn("unable to save cached ZIR code for {s}/{s} to {s}/{s}: {s}", .{3713
3714 pkg_path, file.sub_file_path, cache_path, &digest, @errorName(e),3714 else => |e| return e,
3715 });
3716 return;
3717 },
3718 };3715 };
37193716
3720 mod.lockAndClearFileCompileError(file);3717 mod.lockAndClearFileCompileError(file);
...@@ -3871,7 +3868,7 @@ pub fn astGenFile(mod: *Module, file: *File) !void {...@@ -3871,7 +3868,7 @@ pub fn astGenFile(mod: *Module, file: *File) !void {
3871 .iov_len = file.zir.extra.len * 4,3868 .iov_len = file.zir.extra.len * 4,
3872 },3869 },
3873 };3870 };
3874 cache_file.?.writevAll(&iovecs) catch |err| {3871 cache_file.writevAll(&iovecs) catch |err| {
3875 const pkg_path = file.pkg.root_src_directory.path orelse ".";3872 const pkg_path = file.pkg.root_src_directory.path orelse ".";
3876 const cache_path = cache_directory.path orelse ".";3873 const cache_path = cache_directory.path orelse ".";
3877 log.warn("unable to write cached ZIR code for {s}/{s} to {s}/{s}: {s}", .{3874 log.warn("unable to write cached ZIR code for {s}/{s} to {s}/{s}: {s}", .{
stage1/wasi.c-2
...@@ -497,8 +497,6 @@ uint32_t wasi_snapshot_preview1_fd_read(uint32_t fd, uint32_t iovs, uint32_t iov...@@ -497,8 +497,6 @@ uint32_t wasi_snapshot_preview1_fd_read(uint32_t fd, uint32_t iovs, uint32_t iov
497 size_t read_size = 0;497 size_t read_size = 0;
498 if (fds[fd].stream != NULL)498 if (fds[fd].stream != NULL)
499 read_size = fread(&m[iovs_ptr[i].ptr], 1, iovs_ptr[i].len, fds[fd].stream);499 read_size = fread(&m[iovs_ptr[i].ptr], 1, iovs_ptr[i].len, fds[fd].stream);
500 else
501 panic("unimplemented");
502 size += read_size;500 size += read_size;
503 if (read_size < iovs_ptr[i].len) break;501 if (read_size < iovs_ptr[i].len) break;
504 }502 }