Commit 46388d338a

mlugg <mlugg@mlugg.co.uk>
2024-08-14 09:10:49
InternPool: don't remove outdated types
When a type becomes outdated, there will still be lingering references to the old index -- for instance, any declaration whose value was that type holds a reference to that index. These references may live for an arbitrarily long time in some cases. So, we can't just remove the type from the pool -- the old `Index` must remain valid! Instead, we want to preserve the old `Index`, but avoid it from ever appearing in lookups. (It's okay if analysis of something referencing the old `Index` does weird stuff -- such analysis are guaranteed by the incremental compilation model to always be unreferenced.) So, we use the new `InternPool.putKeyReplace` to replace the shard entry for this index with the newly-created index.
1 parent 978fe68
Changed files (3)
src/Zcu/PerThread.zig
@@ -925,6 +925,7 @@ fn createFileRootStruct(
     pt: Zcu.PerThread,
     file_index: Zcu.File.Index,
     namespace_index: Zcu.Namespace.Index,
+    replace_existing: bool,
 ) Allocator.Error!InternPool.Index {
     const zcu = pt.zcu;
     const gpa = zcu.gpa;
@@ -968,7 +969,7 @@ fn createFileRootStruct(
             .zir_index = tracked_inst,
             .captures = &.{},
         } },
-    })) {
+    }, replace_existing)) {
         .existing => unreachable, // we wouldn't be analysing the file root if this type existed
         .wip => |wip| wip,
     };
@@ -1023,8 +1024,7 @@ fn recreateFileRoot(pt: Zcu.PerThread, file_index: Zcu.File.Index) Zcu.SemaError
         zcu.gpa,
         InternPool.AnalUnit.wrap(.{ .cau = file_root_type_cau }),
     );
-    ip.remove(pt.tid, file_root_type);
-    _ = try pt.createFileRootStruct(file_index, namespace_index);
+    _ = try pt.createFileRootStruct(file_index, namespace_index, true);
 }
 
 /// Re-scan the namespace of a file's root struct type on an incremental update.
@@ -1062,8 +1062,6 @@ fn updateFileNamespace(pt: Zcu.PerThread, file_index: Zcu.File.Index) Allocator.
     try pt.scanNamespace(namespace_index, decls);
 }
 
-/// Regardless of the file status, will create a `Decl` if none exists so that we can track
-/// dependencies and re-analyze when the file becomes outdated.
 fn semaFile(pt: Zcu.PerThread, file_index: Zcu.File.Index) Zcu.SemaError!void {
     const tracy = trace(@src());
     defer tracy.end();
@@ -1083,7 +1081,7 @@ fn semaFile(pt: Zcu.PerThread, file_index: Zcu.File.Index) Zcu.SemaError!void {
         .owner_type = undefined, // set in `createFileRootStruct`
         .file_scope = file_index,
     });
-    const struct_ty = try pt.createFileRootStruct(file_index, new_namespace_index);
+    const struct_ty = try pt.createFileRootStruct(file_index, new_namespace_index, false);
     errdefer zcu.intern_pool.remove(pt.tid, struct_ty);
 
     switch (zcu.comp.cache_use) {
@@ -1153,11 +1151,10 @@ fn semaCau(pt: Zcu.PerThread, cau_index: InternPool.Cau.Index) !SemaCauResult {
             // This declaration has no value so is definitely not a std.builtin type.
             break :ip_index .none;
         },
-        .type => |ty| {
+        .type => {
             // This is an incremental update, and this type is being re-analyzed because it is outdated.
             // The type must be recreated at a new `InternPool.Index`.
-            // Remove it from the InternPool and mark it outdated so that creation sites are re-analyzed.
-            ip.remove(pt.tid, ty);
+            // Mark it outdated so that creation sites are re-analyzed.
             return .{
                 .invalidate_decl_val = true,
                 .invalidate_decl_ref = true,
src/InternPool.zig
@@ -7077,6 +7077,7 @@ fn getOrPutKeyEnsuringAdditionalCapacity(
         const index = entry.acquire();
         if (index == .none) break;
         if (entry.hash != hash) continue;
+        if (ip.isRemoved(index)) continue;
         if (ip.indexToKey(index).eql(key, ip)) return .{ .existing = index };
     }
     shard.mutate.map.mutex.lock();
@@ -7151,6 +7152,43 @@ fn getOrPutKeyEnsuringAdditionalCapacity(
         .map_index = map_index,
     } };
 }
+/// Like `getOrPutKey`, but asserts that the key already exists, and prepares to replace
+/// its shard entry with a new `Index` anyway. After finalizing this, the old index remains
+/// valid (in that `indexToKey` and similar queries will behave as before), but it will
+/// never be returned from a lookup (`getOrPutKey` etc).
+/// This is used by incremental compilation when an existing container type is outdated. In
+/// this case, the type must be recreated at a new `InternPool.Index`, but the old index must
+/// remain valid since now-unreferenced `AnalUnit`s may retain references to it. The old index
+/// will be cleaned up when the `Zcu` undergoes garbage collection.
+fn putKeyReplace(
+    ip: *InternPool,
+    tid: Zcu.PerThread.Id,
+    key: Key,
+) GetOrPutKey {
+    const full_hash = key.hash64(ip);
+    const hash: u32 = @truncate(full_hash >> 32);
+    const shard = &ip.shards[@intCast(full_hash & (ip.shards.len - 1))];
+    shard.mutate.map.mutex.lock();
+    errdefer shard.mutate.map.mutex.unlock();
+    const map = shard.shared.map;
+    const map_mask = map.header().mask();
+    var map_index = hash;
+    while (true) : (map_index += 1) {
+        map_index &= map_mask;
+        const entry = &map.entries[map_index];
+        const index = entry.value;
+        assert(index != .none); // key not present
+        if (entry.hash == hash and ip.indexToKey(index).eql(key, ip)) {
+            break; // we found the entry to replace
+        }
+    }
+    return .{ .new = .{
+        .ip = ip,
+        .tid = tid,
+        .shard = shard,
+        .map_index = map_index,
+    } };
+}
 
 pub fn get(ip: *InternPool, gpa: Allocator, tid: Zcu.PerThread.Id, key: Key) Allocator.Error!Index {
     var gop = try ip.getOrPutKey(gpa, tid, key);
@@ -7990,8 +8028,11 @@ pub fn getUnionType(
     gpa: Allocator,
     tid: Zcu.PerThread.Id,
     ini: UnionTypeInit,
+    /// If it is known that there is an existing type with this key which is outdated,
+    /// this is passed as `true`, and the type is replaced with one at a fresh index.
+    replace_existing: bool,
 ) Allocator.Error!WipNamespaceType.Result {
-    var gop = try ip.getOrPutKey(gpa, tid, .{ .union_type = switch (ini.key) {
+    const key: Key = .{ .union_type = switch (ini.key) {
         .declared => |d| .{ .declared = .{
             .zir_index = d.zir_index,
             .captures = .{ .external = d.captures },
@@ -8000,7 +8041,11 @@ pub fn getUnionType(
             .zir_index = r.zir_index,
             .type_hash = r.type_hash,
         } },
-    } });
+    } };
+    var gop = if (replace_existing)
+        ip.putKeyReplace(tid, key)
+    else
+        try ip.getOrPutKey(gpa, tid, key);
     defer gop.deinit();
     if (gop == .existing) return .{ .existing = gop.existing };
 
@@ -8166,8 +8211,11 @@ pub fn getStructType(
     gpa: Allocator,
     tid: Zcu.PerThread.Id,
     ini: StructTypeInit,
+    /// If it is known that there is an existing type with this key which is outdated,
+    /// this is passed as `true`, and the type is replaced with one at a fresh index.
+    replace_existing: bool,
 ) Allocator.Error!WipNamespaceType.Result {
-    var gop = try ip.getOrPutKey(gpa, tid, .{ .struct_type = switch (ini.key) {
+    const key: Key = .{ .struct_type = switch (ini.key) {
         .declared => |d| .{ .declared = .{
             .zir_index = d.zir_index,
             .captures = .{ .external = d.captures },
@@ -8176,7 +8224,11 @@ pub fn getStructType(
             .zir_index = r.zir_index,
             .type_hash = r.type_hash,
         } },
-    } });
+    } };
+    var gop = if (replace_existing)
+        ip.putKeyReplace(tid, key)
+    else
+        try ip.getOrPutKey(gpa, tid, key);
     defer gop.deinit();
     if (gop == .existing) return .{ .existing = gop.existing };
 
@@ -9200,8 +9252,11 @@ pub fn getEnumType(
     gpa: Allocator,
     tid: Zcu.PerThread.Id,
     ini: EnumTypeInit,
+    /// If it is known that there is an existing type with this key which is outdated,
+    /// this is passed as `true`, and the type is replaced with one at a fresh index.
+    replace_existing: bool,
 ) Allocator.Error!WipEnumType.Result {
-    var gop = try ip.getOrPutKey(gpa, tid, .{ .enum_type = switch (ini.key) {
+    const key: Key = .{ .enum_type = switch (ini.key) {
         .declared => |d| .{ .declared = .{
             .zir_index = d.zir_index,
             .captures = .{ .external = d.captures },
@@ -9210,7 +9265,11 @@ pub fn getEnumType(
             .zir_index = r.zir_index,
             .type_hash = r.type_hash,
         } },
-    } });
+    } };
+    var gop = if (replace_existing)
+        ip.putKeyReplace(tid, key)
+    else
+        try ip.getOrPutKey(gpa, tid, key);
     defer gop.deinit();
     if (gop == .existing) return .{ .existing = gop.existing };
 
src/Sema.zig
@@ -2724,9 +2724,9 @@ fn wrapWipTy(sema: *Sema, wip_ty: anytype) @TypeOf(wip_ty) {
 }
 
 /// Given a type just looked up in the `InternPool`, check whether it is
-/// considered outdated on this update. If so, remove it from the pool
-/// and return `true`.
-fn maybeRemoveOutdatedType(sema: *Sema, ty: InternPool.Index) !bool {
+/// considered outdated on this update. If so, returns `true`, and the
+/// caller must replace the outdated type with a fresh one.
+fn checkOutdatedType(sema: *Sema, ty: InternPool.Index) !bool {
     const pt = sema.pt;
     const zcu = pt.zcu;
     const ip = &zcu.intern_pool;
@@ -2745,7 +2745,6 @@ fn maybeRemoveOutdatedType(sema: *Sema, ty: InternPool.Index) !bool {
     if (!was_outdated) return false;
     _ = zcu.outdated_ready.swapRemove(cau_unit);
     zcu.intern_pool.removeDependenciesForDepender(zcu.gpa, cau_unit);
-    zcu.intern_pool.remove(pt.tid, ty);
     try zcu.markDependeeOutdated(.marked_po, .{ .interned = ty });
     return true;
 }
@@ -2815,14 +2814,14 @@ fn zirStructDecl(
             .captures = captures,
         } },
     };
-    const wip_ty = sema.wrapWipTy(switch (try ip.getStructType(gpa, pt.tid, struct_init)) {
+    const wip_ty = sema.wrapWipTy(switch (try ip.getStructType(gpa, pt.tid, struct_init, false)) {
         .existing => |ty| wip: {
-            if (!try sema.maybeRemoveOutdatedType(ty)) {
+            if (!try sema.checkOutdatedType(ty)) {
                 try sema.declareDependency(.{ .interned = ty });
                 try sema.addTypeReferenceEntry(src, ty);
                 return Air.internedToRef(ty);
             }
-            break :wip (try ip.getStructType(gpa, pt.tid, struct_init)).wip;
+            break :wip (try ip.getStructType(gpa, pt.tid, struct_init, true)).wip;
         },
         .wip => |wip| wip,
     });
@@ -3041,14 +3040,14 @@ fn zirEnumDecl(
             .captures = captures,
         } },
     };
-    const wip_ty = sema.wrapWipTy(switch (try ip.getEnumType(gpa, pt.tid, enum_init)) {
+    const wip_ty = sema.wrapWipTy(switch (try ip.getEnumType(gpa, pt.tid, enum_init, false)) {
         .existing => |ty| wip: {
-            if (!try sema.maybeRemoveOutdatedType(ty)) {
+            if (!try sema.checkOutdatedType(ty)) {
                 try sema.declareDependency(.{ .interned = ty });
                 try sema.addTypeReferenceEntry(src, ty);
                 return Air.internedToRef(ty);
             }
-            break :wip (try ip.getEnumType(gpa, pt.tid, enum_init)).wip;
+            break :wip (try ip.getEnumType(gpa, pt.tid, enum_init, true)).wip;
         },
         .wip => |wip| wip,
     });
@@ -3311,14 +3310,14 @@ fn zirUnionDecl(
             .captures = captures,
         } },
     };
-    const wip_ty = sema.wrapWipTy(switch (try ip.getUnionType(gpa, pt.tid, union_init)) {
+    const wip_ty = sema.wrapWipTy(switch (try ip.getUnionType(gpa, pt.tid, union_init, false)) {
         .existing => |ty| wip: {
-            if (!try sema.maybeRemoveOutdatedType(ty)) {
+            if (!try sema.checkOutdatedType(ty)) {
                 try sema.declareDependency(.{ .interned = ty });
                 try sema.addTypeReferenceEntry(src, ty);
                 return Air.internedToRef(ty);
             }
-            break :wip (try ip.getUnionType(gpa, pt.tid, union_init)).wip;
+            break :wip (try ip.getUnionType(gpa, pt.tid, union_init, true)).wip;
         },
         .wip => |wip| wip,
     });
@@ -3407,7 +3406,7 @@ fn zirOpaqueDecl(
     };
     // No `wrapWipTy` needed as no std.builtin types are opaque.
     const wip_ty = switch (try ip.getOpaqueType(gpa, pt.tid, opaque_init)) {
-        // No `maybeRemoveOutdatedType` as opaque types are never outdated.
+        // No `checkOutdatedType` as opaque types are never outdated.
         .existing => |ty| {
             try sema.addTypeReferenceEntry(src, ty);
             return Air.internedToRef(ty);
@@ -22054,7 +22053,7 @@ fn reifyEnum(
             .zir_index = tracked_inst,
             .type_hash = hasher.final(),
         } },
-    })) {
+    }, false)) {
         .wip => |wip| wip,
         .existing => |ty| {
             try sema.declareDependency(.{ .interned = ty });
@@ -22224,7 +22223,7 @@ fn reifyUnion(
             .zir_index = tracked_inst,
             .type_hash = hasher.final(),
         } },
-    })) {
+    }, false)) {
         .wip => |wip| wip,
         .existing => |ty| {
             try sema.declareDependency(.{ .interned = ty });
@@ -22494,7 +22493,7 @@ fn reifyStruct(
             .zir_index = tracked_inst,
             .type_hash = hasher.final(),
         } },
-    })) {
+    }, false)) {
         .wip => |wip| wip,
         .existing => |ty| {
             try sema.declareDependency(.{ .interned = ty });