Skip to content

Commit

Permalink
Compilation: fix cache hash of incremental builds
Browse files Browse the repository at this point in the history
Without this commit, unrelated test builds using incremental cache mode
(self-hosted, no lld) would end up using the same cache namespace, which
is undesireable since concurrent builds will clobber each other's work.

This happened because of passing the root module to
addModuleToCacheHash. In the case of a test build, the root module
actually does not connect to the rest of the import table. Instead, the
main module needs to be passed, which has "root" in its import table.

The other call to addModuleTableToCacheHash which is in
addNonIncrementalStuffToCacheManifest already correctly passes the main
module.

In the future, I think this problem can be fully addressed by obtaining
an advisory lock on the output binary file. However, even in that case,
it is still valuable to make different compilations use different cache
namespaces lest unrelated compilations suffer from pointless thrashing
rather than being independently edited.
  • Loading branch information
andrewrk committed Jan 2, 2024
1 parent de08fd3 commit 605d81f
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/Compilation.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1374,6 +1374,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
cache.hash.add(options.config.wasi_exec_model);
// TODO audit this and make sure everything is in it

const main_mod = options.main_mod orelse options.root_mod;
const comp = try arena.create(Compilation);
const opt_zcu: ?*Module = if (have_zcu) blk: {
// Pre-open the directory handles for cached ZIR code so that it does not need
Expand Down Expand Up @@ -1420,7 +1421,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
zcu.* = .{
.gpa = gpa,
.comp = comp,
.main_mod = options.main_mod orelse options.root_mod,
.main_mod = main_mod,
.root_mod = options.root_mod,
.std_mod = std_mod,
.global_zir_cache = global_zir_cache,
Expand Down Expand Up @@ -1608,7 +1609,7 @@ pub fn create(gpa: Allocator, arena: Allocator, options: CreateOptions) !*Compil
// do want to namespace different source file names because they are
// likely different compilations and therefore this would be likely to
// cause cache hits.
try addModuleTableToCacheHash(gpa, arena, &hash, options.root_mod, .path_bytes);
try addModuleTableToCacheHash(gpa, arena, &hash, main_mod, .path_bytes);

// In the case of incremental cache mode, this `artifact_directory`
// is computed based on a hash of non-linker inputs, and it is where all
Expand Down

0 comments on commit 605d81f

Please sign in to comment.