authorgravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2021-12-09 21:11:51-08:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2021-12-09 21:11:51-08:00
log77836e08a2384450b5e7933094511b61e3c22140
treebe4d87aea477a27bfb5d32d069df2dbee067407b
parent01cb0bdb8317484cab0f8ee3896c981c851d3bb1
parent3e618f8432533eaa1cf6dcb39bdcb1b2e5bc1e47
signaturebadge-question-mark Signed by PGP key 4AEE18F83AFDEB23

Merge pull request #9930 from PhaseMage/fix-cache-timestamps

Fix isProblematicTimestamp

1 files changed, 75 insertions(+), 55 deletions(-)

src/Cache.zig+75-55
......@@ -1,6 +1,9 @@
11gpa: Allocator,
22manifest_dir: fs.Dir,
33hash: HashHelper = .{},
4/// This value is accessed from multiple threads, protected by mutex.
5recent_problematic_timestamp: i128 = 0,
6mutex: std.Thread.Mutex = .{},
47
58const Cache = @This();
69const std = @import("std");
......@@ -16,7 +19,7 @@ const Compilation = @import("Compilation.zig");
1619const log = std.log.scoped(.cache);
1720
1821/// Be sure to call `Manifest.deinit` after successful initialization.
19pub fn obtain(cache: *const Cache) Manifest {
22pub fn obtain(cache: *Cache) Manifest {
2023 return Manifest{
2124 .cache = cache,
2225 .hash = cache.hash,
......@@ -170,7 +173,7 @@ pub const Lock = struct {
170173/// This is not a general-purpose cache.
171174/// It is designed to be fast and simple, not to withstand attacks using specially-crafted input.
172175pub const Manifest = struct {
173 cache: *const Cache,
176 cache: *Cache,
174177 /// Current state for incremental hashing.
175178 hash: HashHelper,
176179 manifest_file: ?fs.File,
......@@ -181,17 +184,24 @@ pub const Manifest = struct {
181184 /// the same cache directory at the same time.
182185 want_shared_lock: bool = true,
183186 have_exclusive_lock: bool = false,
187 // Indicate that we want isProblematicTimestamp to perform a filesystem write in
188 // order to obtain a problematic timestamp for the next call. Calls after that
189 // will then use the same timestamp, to avoid unnecessary filesystem writes.
190 want_refresh_timestamp: bool = true,
184191 files: std.ArrayListUnmanaged(File) = .{},
185192 hex_digest: [hex_digest_len]u8,
186193 /// Populated when hit() returns an error because of one
187194 /// of the files listed in the manifest.
188195 failed_file_index: ?usize = null,
196 /// Keeps track of the last time we performed a file system write to observe
197 /// what time the file system thinks it is, according to its own granularity.
198 recent_problematic_timestamp: i128 = 0,
189199
190200 /// Add a file as a dependency of process being cached. When `hit` is
191201 /// called, the file's contents will be checked to ensure that it matches
192202 /// the contents from previous times.
193203 ///
194 /// Max file size will be used to determine the amount of space to the file contents
204 /// Max file size will be used to determine the amount of space the file contents
195205 /// are allowed to take up in memory. If max_file_size is null, then the contents
196206 /// will not be loaded into memory.
197207 ///
......@@ -345,6 +355,8 @@ pub const Manifest = struct {
345355 }
346356 }
347357
358 self.want_refresh_timestamp = true;
359
348360 const file_contents = try self.manifest_file.?.reader().readAllAlloc(self.cache.gpa, manifest_file_size_max);
349361 defer self.cache.gpa.free(file_contents);
350362
......@@ -414,7 +426,8 @@ pub const Manifest = struct {
414426
415427 cache_hash_file.stat = actual_stat;
416428
417 if (isProblematicTimestamp(cache_hash_file.stat.mtime)) {
429 if (self.isProblematicTimestamp(cache_hash_file.stat.mtime)) {
430 // The actual file has an unreliable timestamp, force it to be hashed
418431 cache_hash_file.stat.mtime = 0;
419432 cache_hash_file.stat.inode = 0;
420433 }
......@@ -478,6 +491,40 @@ pub const Manifest = struct {
478491 }
479492 }
480493
494 fn isProblematicTimestamp(man: *Manifest, file_time: i128) bool {
495 // If the file_time is prior to the most recent problematic timestamp
496 // then we don't need to access the filesystem.
497 if (file_time < man.recent_problematic_timestamp)
498 return false;
499
500 // Next we will check the globally shared Cache timestamp, which is accessed
501 // from multiple threads.
502 man.cache.mutex.lock();
503 defer man.cache.mutex.unlock();
504
505 // Save the global one to our local one to avoid locking next time.
506 man.recent_problematic_timestamp = man.cache.recent_problematic_timestamp;
507 if (file_time < man.recent_problematic_timestamp)
508 return false;
509
510 // This flag prevents multiple filesystem writes for the same hit() call.
511 if (man.want_refresh_timestamp) {
512 man.want_refresh_timestamp = false;
513
514 var file = man.cache.manifest_dir.createFile("timestamp", .{
515 .read = true,
516 .truncate = true,
517 }) catch return true;
518 defer file.close();
519
520 // Save locally and also save globally (we still hold the global lock).
521 man.recent_problematic_timestamp = (file.stat() catch return true).mtime;
522 man.cache.recent_problematic_timestamp = man.recent_problematic_timestamp;
523 }
524
525 return file_time >= man.recent_problematic_timestamp;
526 }
527
481528 fn populateFileHash(self: *Manifest, ch_file: *File) !void {
482529 log.debug("populateFileHash {s}", .{ch_file.path.?});
483530 const file = try fs.cwd().openFile(ch_file.path.?, .{});
......@@ -485,7 +532,8 @@ pub const Manifest = struct {
485532
486533 ch_file.stat = try file.stat();
487534
488 if (isProblematicTimestamp(ch_file.stat.mtime)) {
535 if (self.isProblematicTimestamp(ch_file.stat.mtime)) {
536 // The actual file has an unreliable timestamp, force it to be hashed
489537 ch_file.stat.mtime = 0;
490538 ch_file.stat.inode = 0;
491539 }
......@@ -520,7 +568,7 @@ pub const Manifest = struct {
520568 }
521569
522570 /// Add a file as a dependency of process being cached, after the initial hash has been
523 /// calculated. This is useful for processes that don't know the all the files that
571 /// calculated. This is useful for processes that don't know all the files that
524572 /// are depended on ahead of time. For example, a source file that can import other files
525573 /// will need to be recompiled if the imported file is changed.
526574 pub fn addFilePostFetch(self: *Manifest, file_path: []const u8, max_file_size: usize) ![]const u8 {
......@@ -741,35 +789,15 @@ fn hashFile(file: fs.File, bin_digest: *[Hasher.mac_length]u8) !void {
741789 hasher.final(bin_digest);
742790}
743791
744/// If the wall clock time, rounded to the same precision as the
745/// mtime, is equal to the mtime, then we cannot rely on this mtime
746/// yet. We will instead save an mtime value that indicates the hash
747/// must be unconditionally computed.
748/// This function recognizes the precision of mtime by looking at trailing
749/// zero bits of the seconds and nanoseconds.
750fn isProblematicTimestamp(fs_clock: i128) bool {
751 const wall_clock = std.time.nanoTimestamp();
752
753 // We have to break the nanoseconds into seconds and remainder nanoseconds
754 // to detect precision of seconds, because looking at the zero bits in base
755 // 2 would not detect precision of the seconds value.
756 const fs_sec = @intCast(i64, @divFloor(fs_clock, std.time.ns_per_s));
757 const fs_nsec = @intCast(i64, @mod(fs_clock, std.time.ns_per_s));
758 var wall_sec = @intCast(i64, @divFloor(wall_clock, std.time.ns_per_s));
759 var wall_nsec = @intCast(i64, @mod(wall_clock, std.time.ns_per_s));
760
761 // First make all the least significant zero bits in the fs_clock, also zero bits in the wall clock.
762 if (fs_nsec == 0) {
763 wall_nsec = 0;
764 if (fs_sec == 0) {
765 wall_sec = 0;
766 } else {
767 wall_sec &= @as(i64, -1) << @intCast(u6, @ctz(i64, fs_sec));
768 }
769 } else {
770 wall_nsec &= @as(i64, -1) << @intCast(u6, @ctz(i64, fs_nsec));
771 }
772 return wall_nsec == fs_nsec and wall_sec == fs_sec;
792// Create/Write a file, close it, then grab its stat.mtime timestamp.
793fn testGetCurrentFileTimestamp() !i128 {
794 var file = try fs.cwd().createFile("test-filetimestamp.tmp", .{
795 .read = true,
796 .truncate = true,
797 });
798 defer file.close();
799
800 return (try file.stat()).mtime;
773801}
774802
775803test "cache file and then recall it" {
......@@ -783,10 +811,11 @@ test "cache file and then recall it" {
783811 const temp_file = "test.txt";
784812 const temp_manifest_dir = "temp_manifest_dir";
785813
786 const ts = std.time.nanoTimestamp();
787814 try cwd.writeFile(temp_file, "Hello, world!\n");
788815
789 while (isProblematicTimestamp(ts)) {
816 // Wait for file timestamps to tick
817 const initial_time = try testGetCurrentFileTimestamp();
818 while ((try testGetCurrentFileTimestamp()) == initial_time) {
790819 std.time.sleep(1);
791820 }
792821
......@@ -838,18 +867,6 @@ test "cache file and then recall it" {
838867 try cwd.deleteFile(temp_file);
839868}
840869
841test "give problematic timestamp" {
842 var fs_clock = std.time.nanoTimestamp();
843 // to make it problematic, we make it only accurate to the second
844 fs_clock = @divTrunc(fs_clock, std.time.ns_per_s);
845 fs_clock *= std.time.ns_per_s;
846 try testing.expect(isProblematicTimestamp(fs_clock));
847}
848
849test "give nonproblematic timestamp" {
850 try testing.expect(!isProblematicTimestamp(std.time.nanoTimestamp() - std.time.ns_per_s));
851}
852
853870test "check that changing a file makes cache fail" {
854871 if (builtin.os.tag == .wasi) {
855872 // https://github.com/ziglang/zig/issues/5437
......@@ -865,10 +882,11 @@ test "check that changing a file makes cache fail" {
865882 try cwd.deleteTree(temp_manifest_dir);
866883 try cwd.deleteTree(temp_file);
867884
868 const ts = std.time.nanoTimestamp();
869885 try cwd.writeFile(temp_file, original_temp_file_contents);
870886
871 while (isProblematicTimestamp(ts)) {
887 // Wait for file timestamps to tick
888 const initial_time = try testGetCurrentFileTimestamp();
889 while ((try testGetCurrentFileTimestamp()) == initial_time) {
872890 std.time.sleep(1);
873891 }
874892
......@@ -982,11 +1000,12 @@ test "Manifest with files added after initial hash work" {
9821000 const temp_file2 = "cache_hash_post_file_test2.txt";
9831001 const temp_manifest_dir = "cache_hash_post_file_manifest_dir";
9841002
985 const ts1 = std.time.nanoTimestamp();
9861003 try cwd.writeFile(temp_file1, "Hello, world!\n");
9871004 try cwd.writeFile(temp_file2, "Hello world the second!\n");
9881005
989 while (isProblematicTimestamp(ts1)) {
1006 // Wait for file timestamps to tick
1007 const initial_time = try testGetCurrentFileTimestamp();
1008 while ((try testGetCurrentFileTimestamp()) == initial_time) {
9901009 std.time.sleep(1);
9911010 }
9921011
......@@ -1031,10 +1050,11 @@ test "Manifest with files added after initial hash work" {
10311050 try testing.expect(mem.eql(u8, &digest1, &digest2));
10321051
10331052 // Modify the file added after initial hash
1034 const ts2 = std.time.nanoTimestamp();
10351053 try cwd.writeFile(temp_file2, "Hello world the second, updated\n");
10361054
1037 while (isProblematicTimestamp(ts2)) {
1055 // Wait for file timestamps to tick
1056 const initial_time2 = try testGetCurrentFileTimestamp();
1057 while ((try testGetCurrentFileTimestamp()) == initial_time2) {
10381058 std.time.sleep(1);
10391059 }
10401060