Skip to content

Commit

Permalink
frontend: incremental progress.
Browse files Browse the repository at this point in the history 
This fixes the regressions introduced by ziglang#20964. It also cleans up the
"outdated file root" mechanism by virtue of deleting it. We now detect
outdated file roots just after updating ZIR refs, and re-scan their
namespaces.

@jacobly0, this deletes some asserts in `InternPool` which didn't look
appropriate to me, in order to deal with ziglang#20697.
  • Loading branch information
@mlugg
mlugg committed Aug 12, 2024
1 parent fc29240 commit 3f89809
Show file tree
Hide file tree
Showing 5 changed files with 228 additions and 254 deletions.
17 changes: 8 additions & 9 deletions src/Compilation.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3599,10 +3599,9 @@ fn performAllTheWorkInner(
// Pre-load these things from our single-threaded context since they
// will be needed by the worker threads.
const path_digest = zcu.filePathDigest(file_index);
const old_root_type = zcu.fileRootType(file_index);
const file = zcu.fileByIndex(file_index);
comp.thread_pool.spawnWgId(&astgen_wait_group, workerAstGenFile, .{
comp, file, file_index, path_digest, old_root_type, zir_prog_node, &astgen_wait_group, .root,
comp, file, file_index, path_digest, zir_prog_node, &astgen_wait_group, .root,
});
}
}
Expand Down Expand Up @@ -3640,6 +3639,7 @@ fn performAllTheWorkInner(
}
try reportMultiModuleErrors(pt);
try zcu.flushRetryableFailures();

zcu.sema_prog_node = main_progress_node.start("Semantic Analysis", 0);
zcu.codegen_prog_node = main_progress_node.start("Code Generation", 0);
}
Expand Down Expand Up @@ -4291,7 +4291,6 @@ fn workerAstGenFile(
file: *Zcu.File,
file_index: Zcu.File.Index,
path_digest: Cache.BinDigest,
old_root_type: InternPool.Index,
prog_node: std.Progress.Node,
wg: *WaitGroup,
src: Zcu.AstGenSrc,
Expand All @@ -4300,7 +4299,7 @@ fn workerAstGenFile(
defer child_prog_node.end();

const pt: Zcu.PerThread = .{ .zcu = comp.module.?, .tid = @enumFromInt(tid) };
pt.astGenFile(file, path_digest, old_root_type) catch |err| switch (err) {
pt.astGenFile(file, path_digest) catch |err| switch (err) {
error.AnalysisFail => return,
else => {
file.status = .retryable_failure;
Expand Down Expand Up @@ -4331,7 +4330,7 @@ fn workerAstGenFile(
// `@import("builtin")` is handled specially.
if (mem.eql(u8, import_path, "builtin")) continue;

const import_result, const imported_path_digest, const imported_root_type = blk: {
const import_result, const imported_path_digest = blk: {
comp.mutex.lock();
defer comp.mutex.unlock();

Expand All @@ -4346,8 +4345,7 @@ fn workerAstGenFile(
comp.appendFileSystemInput(fsi, res.file.mod.root, res.file.sub_file_path) catch continue;
};
const imported_path_digest = pt.zcu.filePathDigest(res.file_index);
const imported_root_type = pt.zcu.fileRootType(res.file_index);
break :blk .{ res, imported_path_digest, imported_root_type };
break :blk .{ res, imported_path_digest };
};
if (import_result.is_new) {
log.debug("AstGen of {s} has import '{s}'; queuing AstGen of {s}", .{
Expand All @@ -4358,7 +4356,7 @@ fn workerAstGenFile(
.import_tok = item.data.token,
} };
comp.thread_pool.spawnWgId(wg, workerAstGenFile, .{
comp, import_result.file, import_result.file_index, imported_path_digest, imported_root_type, prog_node, wg, sub_src,
comp, import_result.file, import_result.file_index, imported_path_digest, prog_node, wg, sub_src,
});
}
}
Expand Down Expand Up @@ -6451,7 +6449,8 @@ fn buildOutputFromZig(

try comp.updateSubCompilation(sub_compilation, misc_task_tag, prog_node);

assert(out.* == null);
// Under incremental compilation, `out` may already be populated from a prior update.
assert(out.* == null or comp.incremental);
out.* = try sub_compilation.toCrtFile();
}

Expand Down
2 changes: 0 additions & 2 deletions src/InternPool.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,6 @@ const Local = struct {

pub fn view(mutable: Mutable) View {
const capacity = mutable.list.header().capacity;
assert(capacity > 0); // optimizes `MultiArrayList.Slice.items`
return .{
.bytes = mutable.list.bytes,
.len = mutable.mutate.len,
Expand Down Expand Up @@ -999,7 +998,6 @@ const Local = struct {

pub fn view(list: ListSelf) View {
const capacity = list.header().capacity;
assert(capacity > 0); // optimizes `MultiArrayList.Slice.items`
return .{
.bytes = list.bytes,
.len = capacity,
Expand Down
3 changes: 1 addition & 2 deletions src/Sema.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6006,8 +6006,7 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
return sema.fail(&child_block, src, "C import failed: {s}", .{@errorName(err)});

const path_digest = zcu.filePathDigest(result.file_index);
const old_root_type = zcu.fileRootType(result.file_index);
pt.astGenFile(result.file, path_digest, old_root_type) catch |err|
pt.astGenFile(result.file, path_digest) catch |err|
return sema.fail(&child_block, src, "C import failed: {s}", .{@errorName(err)});

// TODO: register some kind of dependency on the file.
Expand Down
111 changes: 36 additions & 75 deletions src/Zcu.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,6 @@ outdated: std.AutoArrayHashMapUnmanaged(AnalUnit, u32) = .{},
/// Such `AnalUnit`s are ready for immediate re-analysis.
/// See `findOutdatedToAnalyze` for details.
outdated_ready: std.AutoArrayHashMapUnmanaged(AnalUnit, void) = .{},
/// This contains a set of struct types whose corresponding `Cau` may not be in
/// `outdated`, but are the root types of files which have updated source and
/// thus must be re-analyzed. If such a type is only in this set, the struct type
/// index may be preserved (only the namespace might change). If its owned `Cau`
/// is also outdated, the struct type index must be recreated.
outdated_file_root: std.AutoArrayHashMapUnmanaged(InternPool.Index, void) = .{},
/// This contains a list of AnalUnit whose analysis or codegen failed, but the
/// failure was something like running out of disk space, and trying again may
/// succeed. On the next update, we will flush this list, marking all members of
Expand Down Expand Up @@ -2148,7 +2142,6 @@ pub fn deinit(zcu: *Zcu) void {
zcu.potentially_outdated.deinit(gpa);
zcu.outdated.deinit(gpa);
zcu.outdated_ready.deinit(gpa);
zcu.outdated_file_root.deinit(gpa);
zcu.retryable_failures.deinit(gpa);

zcu.test_functions.deinit(gpa);
Expand Down Expand Up @@ -2355,8 +2348,6 @@ fn markTransitiveDependersPotentiallyOutdated(zcu: *Zcu, maybe_outdated: AnalUni
pub fn findOutdatedToAnalyze(zcu: *Zcu) Allocator.Error!?AnalUnit {
if (!zcu.comp.incremental) return null;

if (true) @panic("TODO: findOutdatedToAnalyze");

if (zcu.outdated.count() == 0 and zcu.potentially_outdated.count() == 0) {
log.debug("findOutdatedToAnalyze: no outdated depender", .{});
return null;
Expand All @@ -2381,87 +2372,57 @@ pub fn findOutdatedToAnalyze(zcu: *Zcu) Allocator.Error!?AnalUnit {
return zcu.outdated_ready.keys()[0];
}

// Next, we will see if there is any outdated file root which was not in
// `outdated`. This set will be small (number of files changed in this
// update), so it's alright for us to just iterate here.
for (zcu.outdated_file_root.keys()) |file_decl| {
const decl_depender = AnalUnit.wrap(.{ .decl = file_decl });
if (zcu.outdated.contains(decl_depender)) {
// Since we didn't hit this in the first loop, this Decl must have
// pending dependencies, so is ineligible.
continue;
}
if (zcu.potentially_outdated.contains(decl_depender)) {
// This Decl's struct may or may not need to be recreated depending
// on whether it is outdated. If we analyzed it now, we would have
// to assume it was outdated and recreate it!
continue;
}
log.debug("findOutdatedToAnalyze: outdated file root decl '{d}'", .{file_decl});
return decl_depender;
}

// There is no single AnalUnit which is ready for re-analysis. Instead, we
// must assume that some Decl with PO dependencies is outdated - e.g. in the
// above example we arbitrarily pick one of A or B. We should select a Decl,
// since a Decl is definitely responsible for the loop in the dependency
// graph (since you can't depend on a runtime function analysis!).
// There is no single AnalUnit which is ready for re-analysis. Instead, we must assume that some
// Cau with PO dependencies is outdated -- e.g. in the above example we arbitrarily pick one of
// A or B. We should select a Cau, since a Cau is definitely responsible for the loop in the
// dependency graph (since IES dependencies can't have loops). We should also, of course, not
// select a Cau owned by a `comptime` declaration, since you can't depend on those!

// The choice of this Decl could have a big impact on how much total
// analysis we perform, since if analysis concludes its tyval is unchanged,
// then other PO AnalUnit may be resolved as up-to-date. To hopefully avoid
// doing too much work, let's find a Decl which the most things depend on -
// the idea is that this will resolve a lot of loops (but this is only a
// heuristic).
// The choice of this Cau could have a big impact on how much total analysis we perform, since
// if analysis concludes any dependencies on its result are up-to-date, then other PO AnalUnit
// may be resolved as up-to-date. To hopefully avoid doing too much work, let's find a Decl
// which the most things depend on - the idea is that this will resolve a lot of loops (but this
// is only a heuristic).

log.debug("findOutdatedToAnalyze: no trivial ready, using heuristic; {d} outdated, {d} PO", .{
zcu.outdated.count(),
zcu.potentially_outdated.count(),
});

const Decl = {};

var chosen_decl_idx: ?Decl.Index = null;
var chosen_decl_dependers: u32 = undefined;

for (zcu.outdated.keys()) |depender| {
const decl_index = switch (depender.unwrap()) {
.decl => |d| d,
.func => continue,
};

var n: u32 = 0;
var it = zcu.intern_pool.dependencyIterator(.{ .decl_val = decl_index });
while (it.next()) |_| n += 1;
const ip = &zcu.intern_pool;

if (chosen_decl_idx == null or n > chosen_decl_dependers) {
chosen_decl_idx = decl_index;
chosen_decl_dependers = n;
}
}
var chosen_cau: ?InternPool.Cau.Index = null;
var chosen_cau_dependers: u32 = undefined;

for (zcu.potentially_outdated.keys()) |depender| {
const decl_index = switch (depender.unwrap()) {
.decl => |d| d,
.func => continue,
};
inline for (.{ zcu.outdated.keys(), zcu.potentially_outdated.keys() }) |outdated_units| {
for (outdated_units) |unit| {
const cau = switch (unit.unwrap()) {
.cau => |cau| cau,
.func => continue, // a `func` definitely can't be causing the loop so it is a bad choice
};
const cau_owner = ip.getCau(cau).owner;

var n: u32 = 0;
var it = zcu.intern_pool.dependencyIterator(.{ .decl_val = decl_index });
while (it.next()) |_| n += 1;
var n: u32 = 0;
var it = ip.dependencyIterator(switch (cau_owner.unwrap()) {
.none => continue, // there can be no dependencies on this `Cau` so it is a terrible choice
.type => |ty| .{ .interned = ty },
.nav => |nav| .{ .nav_val = nav },
});
while (it.next()) |_| n += 1;

if (chosen_decl_idx == null or n > chosen_decl_dependers) {
chosen_decl_idx = decl_index;
chosen_decl_dependers = n;
if (chosen_cau == null or n > chosen_cau_dependers) {
chosen_cau = cau;
chosen_cau_dependers = n;
}
}
}

log.debug("findOutdatedToAnalyze: heuristic returned Decl {d} ({d} dependers)", .{
chosen_decl_idx.?,
chosen_decl_dependers,
log.debug("findOutdatedToAnalyze: heuristic returned Cau {d} ({d} dependers)", .{
@intFromEnum(chosen_cau.?),
chosen_cau_dependers,
});

return AnalUnit.wrap(.{ .decl = chosen_decl_idx.? });
return AnalUnit.wrap(.{ .cau = chosen_cau.? });
}

/// During an incremental update, before semantic analysis, call this to flush all values from
Expand Down Expand Up @@ -2583,7 +2544,7 @@ pub fn mapOldZirToNew(
break :inst unnamed_tests.items[unnamed_test_idx];
},
_ => inst: {
const name_nts = new_decl.name.toString(old_zir).?;
const name_nts = new_decl.name.toString(new_zir).?;
const name = new_zir.nullTerminatedString(name_nts);
if (new_decl.name.isNamedTest(new_zir)) {
break :inst named_tests.get(name) orelse continue;
Expand Down
Loading

0 comments on commit 3f89809

Please sign in to comment.