Skip to content

Commit 8b75cfe

Browse files
committed
fix(cache): handle missing cache hits when chaining two run steps
fixes #19817 This improves the efficiency of the cache when chaining muliple commands like const step1 = b.addRunArtifact(tool_fast); step1.addFileArg(b.path("src/input.c")); const output1 = step1.addOutputFileArg("output1.h"); const step2 = b.addRunArtifact(tool_slow); step2.addFileArg(output1); const chained_output = step2.addOutputFileArg("output2.h"); assume that step2 takes much long time than step1 if we make a change to "src/input.c" which produces an identical "output1.h" as a previous input, one would expect step2 not to rerun as the cached output2.h only depends on the content of output1.h However, this does not work yet as the hash of src/input.c leaks into the file name of the cached output1.h, which the second run step interprets as a different cache key. Not using the ".zig-build/o/{HASH}" part of the file name in the hash key fixes this.
1 parent afdd043 commit 8b75cfe

File tree

4 files changed

+29
-16
lines changed

4 files changed

+29
-16
lines changed

lib/std/Build.zig

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2314,6 +2314,9 @@ pub const LazyPath = union(enum) {
23142314

23152315
/// Applied after `up`.
23162316
sub_path: []const u8 = "",
2317+
2318+
/// If set, the file is hashed only on this suffix, not the full absolute path
2319+
content_hash_name: ?[]const u8 = null,
23172320
},
23182321

23192322
/// An absolute path or a path relative to the current working directory of
@@ -2503,7 +2506,9 @@ pub const LazyPath = union(enum) {
25032506
}
25042507
}
25052508

2506-
return file_path.join(src_builder.allocator, gen.sub_path) catch @panic("OOM");
2509+
var item = file_path.join(src_builder.allocator, gen.sub_path) catch @panic("OOM");
2510+
item.content_hash_name = gen.content_hash_name;
2511+
return item;
25072512
},
25082513
.dependency => |dep| return .{
25092514
.root_dir = dep.dependency.builder.build_root,
@@ -2543,6 +2548,7 @@ pub const LazyPath = union(enum) {
25432548
.file = gen.file,
25442549
.up = gen.up,
25452550
.sub_path = dupePathInner(allocator, gen.sub_path),
2551+
.content_hash_name = if (gen.content_hash_name) |name| Build.dupeInner(allocator, name) else null,
25462552
} },
25472553
.dependency => |dep| .{ .dependency = .{
25482554
.dependency = dep.dependency,

lib/std/Build/Cache.zig

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ pub const Manifest = struct {
404404
});
405405
errdefer gpa.free(resolved_path);
406406
const prefixed_path = try m.cache.findPrefixResolved(resolved_path);
407-
return addFileInner(m, prefixed_path, handle, max_file_size);
407+
return addFileInner(m, prefixed_path, handle, max_file_size, path.content_hash_name);
408408
}
409409

410410
/// Deprecated; use `addFilePath`.
@@ -416,10 +416,10 @@ pub const Manifest = struct {
416416
const prefixed_path = try self.cache.findPrefix(file_path);
417417
errdefer gpa.free(prefixed_path.sub_path);
418418

419-
return addFileInner(self, prefixed_path, null, max_file_size);
419+
return addFileInner(self, prefixed_path, null, max_file_size, null);
420420
}
421421

422-
fn addFileInner(self: *Manifest, prefixed_path: PrefixedPath, handle: ?fs.File, max_file_size: ?usize) usize {
422+
fn addFileInner(self: *Manifest, prefixed_path: PrefixedPath, handle: ?fs.File, max_file_size: ?usize, content_hash_name: ?[]const u8) usize {
423423
const gop = self.files.getOrPutAssumeCapacityAdapted(prefixed_path, FilesAdapter{});
424424
if (gop.found_existing) {
425425
self.cache.gpa.free(prefixed_path.sub_path);
@@ -436,8 +436,13 @@ pub const Manifest = struct {
436436
.handle = handle,
437437
};
438438

439-
self.hash.add(prefixed_path.prefix);
440-
self.hash.addBytes(prefixed_path.sub_path);
439+
if (content_hash_name) |name| {
440+
self.hash.add(@as(u8, '?'));
441+
self.hash.addBytes(name);
442+
} else {
443+
self.hash.add(prefixed_path.prefix);
444+
self.hash.addBytes(prefixed_path.sub_path);
445+
}
441446

442447
return gop.index;
443448
}
@@ -715,15 +720,13 @@ pub const Manifest = struct {
715720

716721
if (file_path.len == 0) return error.InvalidFormat;
717722

723+
const prefixed_path: PrefixedPath = .{
724+
.prefix = prefix,
725+
.sub_path = file_path, // expires with file_contents
726+
};
718727
const cache_hash_file = f: {
719-
const prefixed_path: PrefixedPath = .{
720-
.prefix = prefix,
721-
.sub_path = file_path, // expires with file_contents
722-
};
723728
if (idx < input_file_count) {
724729
const file = &self.files.keys()[idx];
725-
if (!file.prefixed_path.eql(prefixed_path))
726-
return error.InvalidFormat;
727730

728731
file.stat = .{
729732
.size = stat_size,
@@ -779,11 +782,13 @@ pub const Manifest = struct {
779782
} };
780783
return error.CacheCheckFailed;
781784
};
785+
786+
const name_match = pp.eql(prefixed_path);
782787
const size_match = actual_stat.size == cache_hash_file.stat.size;
783788
const mtime_match = actual_stat.mtime.nanoseconds == cache_hash_file.stat.mtime.nanoseconds;
784789
const inode_match = actual_stat.inode == cache_hash_file.stat.inode;
785790

786-
if (!size_match or !mtime_match or !inode_match) {
791+
if (!name_match or !size_match or !mtime_match or !inode_match) {
787792
cache_hash_file.stat = .{
788793
.size = actual_stat.size,
789794
.mtime = actual_stat.mtime,

lib/std/Build/Cache/Path.zig

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@ root_dir: Cache.Directory,
1111
/// The path, relative to the root dir, that this `Path` represents.
1212
/// Empty string means the root_dir is the path.
1313
sub_path: []const u8 = "",
14+
content_hash_name: ?[]const u8 = null,
1415

1516
pub fn clone(p: Path, arena: Allocator) Allocator.Error!Path {
1617
return .{
1718
.root_dir = try p.root_dir.clone(arena),
1819
.sub_path = try arena.dupe(u8, p.sub_path),
20+
.content_hash_name = if (p.content_hash_name) |name| try arena.dupe(u8, name) else null,
1921
};
2022
}
2123

lib/std/Build/Step/Run.zig

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ pub fn addPrefixedOutputFileArg(
300300
run.setName(b.fmt("{s} ({s})", .{ run.step.name, basename }));
301301
}
302302

303-
return .{ .generated = .{ .file = &output.generated_file } };
303+
return .{ .generated = .{ .file = &output.generated_file, .content_hash_name = output.basename } };
304304
}
305305

306306
/// Appends an input file to the command line arguments.
@@ -885,8 +885,8 @@ fn make(step: *Step, options: Step.MakeOptions) !void {
885885
man.hash.addBytes(bytes);
886886
},
887887
.lazy_path => |lazy_path| {
888-
const file_path = lazy_path.getPath2(b, step);
889-
_ = try man.addFile(file_path, null);
888+
const file_path = lazy_path.getPath3(b, step);
889+
_ = try man.addFilePath(file_path, null);
890890
},
891891
.none => {},
892892
}

0 commit comments

Comments
 (0)