Commit f290b54f89

Jacob Young <jacobly0@users.noreply.github.com>
2024-07-10 16:43:51
InternPool: make `files` more thread-safe
1 parent 8f29243
src/Zcu/PerThread.zig
@@ -1386,15 +1386,16 @@ pub fn importPkg(pt: Zcu.PerThread, mod: *Module) !Zcu.ImportFileResult {
     }
 
     const ip = &zcu.intern_pool;
-    try ip.files.ensureUnusedCapacity(gpa, 1);
-
     if (mod.builtin_file) |builtin_file| {
-        const file_index = try ip.createFile(gpa, pt.tid, builtin_file);
+        const path_digest = Zcu.computePathDigest(zcu, mod, builtin_file.sub_file_path);
+        const file_index = try ip.createFile(gpa, pt.tid, .{
+            .bin_digest = path_digest,
+            .file = builtin_file,
+            .root_decl = .none,
+        });
         keep_resolved_path = true; // It's now owned by import_table.
         gop.value_ptr.* = file_index;
         try builtin_file.addReference(zcu, .{ .root = mod });
-        const path_digest = Zcu.computePathDigest(zcu, mod, builtin_file.sub_file_path);
-        ip.files.putAssumeCapacityNoClobber(path_digest, .none);
         return .{
             .file = builtin_file,
             .file_index = file_index,
@@ -1409,7 +1410,12 @@ pub fn importPkg(pt: Zcu.PerThread, mod: *Module) !Zcu.ImportFileResult {
     const new_file = try gpa.create(Zcu.File);
     errdefer gpa.destroy(new_file);
 
-    const new_file_index = try ip.createFile(gpa, pt.tid, new_file);
+    const path_digest = zcu.computePathDigest(mod, sub_file_path);
+    const new_file_index = try ip.createFile(gpa, pt.tid, .{
+        .bin_digest = path_digest,
+        .file = new_file,
+        .root_decl = .none,
+    });
     keep_resolved_path = true; // It's now owned by import_table.
     gop.value_ptr.* = new_file_index;
     new_file.* = .{
@@ -1425,10 +1431,7 @@ pub fn importPkg(pt: Zcu.PerThread, mod: *Module) !Zcu.ImportFileResult {
         .mod = mod,
     };
 
-    const path_digest = zcu.computePathDigest(mod, sub_file_path);
-
     try new_file.addReference(zcu, .{ .root = mod });
-    ip.files.putAssumeCapacityNoClobber(path_digest, .none);
     return .{
         .file = new_file,
         .file_index = new_file_index,
@@ -1489,8 +1492,6 @@ pub fn importFile(
 
     const ip = &zcu.intern_pool;
 
-    try ip.files.ensureUnusedCapacity(gpa, 1);
-
     const new_file = try gpa.create(Zcu.File);
     errdefer gpa.destroy(new_file);
 
@@ -1515,7 +1516,12 @@ pub fn importFile(
         resolved_root_path, resolved_path, sub_file_path, import_string,
     });
 
-    const new_file_index = try ip.createFile(gpa, pt.tid, new_file);
+    const path_digest = zcu.computePathDigest(mod, sub_file_path);
+    const new_file_index = try ip.createFile(gpa, pt.tid, .{
+        .bin_digest = path_digest,
+        .file = new_file,
+        .root_decl = .none,
+    });
     keep_resolved_path = true; // It's now owned by import_table.
     gop.value_ptr.* = new_file_index;
     new_file.* = .{
@@ -1531,8 +1537,6 @@ pub fn importFile(
         .mod = mod,
     };
 
-    const path_digest = zcu.computePathDigest(mod, sub_file_path);
-    ip.files.putAssumeCapacityNoClobber(path_digest, .none);
     return .{
         .file = new_file,
         .file_index = new_file_index,
src/Compilation.zig
@@ -2784,7 +2784,7 @@ const Header = extern struct {
         first_dependency_len: u32,
         dep_entries_len: u32,
         free_dep_entries_len: u32,
-        files_len: u32,
+        //files_len: u32,
     },
 };
 
@@ -2813,7 +2813,7 @@ pub fn saveState(comp: *Compilation) !void {
                 .first_dependency_len = @intCast(ip.first_dependency.count()),
                 .dep_entries_len = @intCast(ip.dep_entries.items.len),
                 .free_dep_entries_len = @intCast(ip.free_dep_entries.items.len),
-                .files_len = @intCast(ip.files.entries.len),
+                //.files_len = @intCast(ip.files.entries.len),
             },
         };
         addBuf(&bufs_list, &bufs_len, mem.asBytes(&header));
@@ -2838,8 +2838,8 @@ pub fn saveState(comp: *Compilation) !void {
         addBuf(&bufs_list, &bufs_len, mem.sliceAsBytes(ip.dep_entries.items));
         addBuf(&bufs_list, &bufs_len, mem.sliceAsBytes(ip.free_dep_entries.items));
 
-        addBuf(&bufs_list, &bufs_len, mem.sliceAsBytes(ip.files.keys()));
-        addBuf(&bufs_list, &bufs_len, mem.sliceAsBytes(ip.files.values()));
+        //addBuf(&bufs_list, &bufs_len, mem.sliceAsBytes(ip.files.keys()));
+        //addBuf(&bufs_list, &bufs_len, mem.sliceAsBytes(ip.files.values()));
 
         // TODO: compilation errors
         // TODO: namespaces
src/InternPool.zig
@@ -59,17 +59,6 @@ dep_entries: std.ArrayListUnmanaged(DepEntry) = .{},
 /// garbage collection pass.
 free_dep_entries: std.ArrayListUnmanaged(DepEntry.Index) = .{},
 
-/// Elements are ordered identically to the `import_table` field of `Zcu`.
-///
-/// Unlike `import_table`, this data is serialized as part of incremental
-/// compilation state.
-///
-/// Key is the hash of the path to this file, used to store
-/// `InternPool.TrackedInst`.
-///
-/// Value is the `Decl` of the struct that represents this `File`.
-files: std.AutoArrayHashMapUnmanaged(Cache.BinDigest, OptionalDeclIndex) = .{},
-
 /// Whether a multi-threaded intern pool is useful.
 /// Currently `false` until the intern pool is actually accessed
 /// from multiple threads to reduce the cost of this data structure.
@@ -346,7 +335,7 @@ const Local = struct {
         extra: Extra,
         limbs: Limbs,
         strings: Strings,
-        files: Files,
+        files: List(File),
 
         decls: Decls,
         namespaces: Namespaces,
@@ -367,7 +356,6 @@ const Local = struct {
         else => @compileError("unsupported host"),
     };
     const Strings = List(struct { u8 });
-    const Files = List(struct { *Zcu.File });
 
     const decls_bucket_width = 8;
     const decls_bucket_mask = (1 << decls_bucket_width) - 1;
@@ -600,7 +588,7 @@ const Local = struct {
             const View = std.MultiArrayList(Elem);
 
             /// Must be called when accessing from another thread.
-            fn acquire(list: *const ListSelf) ListSelf {
+            pub fn acquire(list: *const ListSelf) ListSelf {
                 return .{ .bytes = @atomicLoad([*]align(@alignOf(Elem)) u8, &list.bytes, .acquire) };
             }
             fn release(list: *ListSelf, new_list: ListSelf) void {
@@ -614,7 +602,7 @@ const Local = struct {
                 return @ptrFromInt(@intFromPtr(list.bytes) - bytes_offset);
             }
 
-            fn view(list: ListSelf) View {
+            pub fn view(list: ListSelf) View {
                 const capacity = list.header().capacity;
                 assert(capacity > 0); // optimizes `MultiArrayList.Slice.items`
                 return .{
@@ -675,7 +663,16 @@ const Local = struct {
         };
     }
 
-    pub fn getMutableFiles(local: *Local, gpa: std.mem.Allocator) Files.Mutable {
+    /// Elements are ordered identically to the `import_table` field of `Zcu`.
+    ///
+    /// Unlike `import_table`, this data is serialized as part of incremental
+    /// compilation state.
+    ///
+    /// Key is the hash of the path to this file, used to store
+    /// `InternPool.TrackedInst`.
+    ///
+    /// Value is the `Decl` of the struct that represents this `File`.
+    pub fn getMutableFiles(local: *Local, gpa: std.mem.Allocator) List(File).Mutable {
         return .{
             .gpa = gpa,
             .arena = &local.mutate.arena,
@@ -957,7 +954,7 @@ pub const FileIndex = enum(u32) {
                 unwrapped.index);
         }
     };
-    fn unwrap(file_index: FileIndex, ip: *const InternPool) Unwrapped {
+    pub fn unwrap(file_index: FileIndex, ip: *const InternPool) Unwrapped {
         return .{
             .tid = @enumFromInt(@intFromEnum(file_index) >> ip.tid_shift_32 & ip.getTidMask()),
             .index = @intFromEnum(file_index) & ip.getIndexMask(u32),
@@ -965,6 +962,12 @@ pub const FileIndex = enum(u32) {
     }
 };
 
+const File = struct {
+    bin_digest: Cache.BinDigest,
+    file: *Zcu.File,
+    root_decl: OptionalDeclIndex,
+};
+
 /// An index into `strings`.
 pub const String = enum(u32) {
     /// An empty string.
@@ -5237,7 +5240,7 @@ pub fn init(ip: *InternPool, gpa: Allocator, available_threads: usize) !void {
             .extra = Local.Extra.empty,
             .limbs = Local.Limbs.empty,
             .strings = Local.Strings.empty,
-            .files = Local.Files.empty,
+            .files = Local.List(File).empty,
 
             .decls = Local.Decls.empty,
             .namespaces = Local.Namespaces.empty,
@@ -5321,8 +5324,6 @@ pub fn deinit(ip: *InternPool, gpa: Allocator) void {
     ip.dep_entries.deinit(gpa);
     ip.free_dep_entries.deinit(gpa);
 
-    ip.files.deinit(gpa);
-
     gpa.free(ip.shards);
     for (ip.locals) |*local| {
         const buckets_len = local.mutate.namespaces.buckets_list.len;
@@ -9790,21 +9791,21 @@ pub fn destroyNamespace(
 pub fn filePtr(ip: *InternPool, file_index: FileIndex) *Zcu.File {
     const file_index_unwrapped = file_index.unwrap(ip);
     const files = ip.getLocalShared(file_index_unwrapped.tid).files.acquire();
-    return files.view().items(.@"0")[file_index_unwrapped.index];
+    return files.view().items(.file)[file_index_unwrapped.index];
 }
 
 pub fn createFile(
     ip: *InternPool,
     gpa: Allocator,
     tid: Zcu.PerThread.Id,
-    file: *Zcu.File,
+    file: File,
 ) Allocator.Error!FileIndex {
     const files = ip.getLocal(tid).getMutableFiles(gpa);
     const file_index_unwrapped: FileIndex.Unwrapped = .{
         .tid = tid,
         .index = files.mutate.len,
     };
-    try files.append(.{file});
+    try files.append(file);
     return file_index_unwrapped.wrap(ip);
 }
 
src/Zcu.zig
@@ -2406,8 +2406,7 @@ pub fn deinit(zcu: *Zcu) void {
     for (zcu.import_table.keys()) |key| {
         gpa.free(key);
     }
-    for (0..zcu.import_table.entries.len) |file_index_usize| {
-        const file_index: File.Index = @enumFromInt(file_index_usize);
+    for (zcu.import_table.values()) |file_index| {
         pt.destroyFile(file_index);
     }
     zcu.import_table.deinit(gpa);
@@ -3537,23 +3536,28 @@ pub fn resolveReferences(zcu: *Zcu) !std.AutoHashMapUnmanaged(AnalUnit, Resolved
     return result;
 }
 
-pub fn fileByIndex(zcu: *Zcu, i: File.Index) *File {
-    const ip = &zcu.intern_pool;
-    return ip.filePtr(i);
+pub fn fileByIndex(zcu: *Zcu, file_index: File.Index) *File {
+    return zcu.intern_pool.filePtr(file_index);
 }
 
 /// Returns the `Decl` of the struct that represents this `File`.
-pub fn fileRootDecl(zcu: *const Zcu, i: File.Index) Decl.OptionalIndex {
+pub fn fileRootDecl(zcu: *const Zcu, file_index: File.Index) Decl.OptionalIndex {
     const ip = &zcu.intern_pool;
-    return ip.files.values()[@intFromEnum(i)];
+    const file_index_unwrapped = file_index.unwrap(ip);
+    const files = ip.getLocalShared(file_index_unwrapped.tid).files.acquire();
+    return files.view().items(.root_decl)[file_index_unwrapped.index];
 }
 
-pub fn setFileRootDecl(zcu: *Zcu, i: File.Index, root_decl: Decl.OptionalIndex) void {
+pub fn setFileRootDecl(zcu: *Zcu, file_index: File.Index, root_decl: Decl.OptionalIndex) void {
     const ip = &zcu.intern_pool;
-    ip.files.values()[@intFromEnum(i)] = root_decl;
+    const file_index_unwrapped = file_index.unwrap(ip);
+    const files = ip.getLocalShared(file_index_unwrapped.tid).files.acquire();
+    files.view().items(.root_decl)[file_index_unwrapped.index] = root_decl;
 }
 
-pub fn filePathDigest(zcu: *const Zcu, i: File.Index) Cache.BinDigest {
+pub fn filePathDigest(zcu: *const Zcu, file_index: File.Index) Cache.BinDigest {
     const ip = &zcu.intern_pool;
-    return ip.files.keys()[@intFromEnum(i)];
+    const file_index_unwrapped = file_index.unwrap(ip);
+    const files = ip.getLocalShared(file_index_unwrapped.tid).files.acquire();
+    return files.view().items(.bin_digest)[file_index_unwrapped.index];
 }