From f290b54f891a67af456529da0f4f824a1e27b4ef Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Wed, 10 Jul 2024 10:43:51 -0400 Subject: [PATCH] InternPool: make `files` more thread-safe --- src/Compilation.zig | 8 ++++---- src/InternPool.zig | 47 ++++++++++++++++++++++--------------------- src/Zcu.zig | 26 ++++++++++++++---------- src/Zcu/PerThread.zig | 32 ++++++++++++++++------------- 4 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/Compilation.zig b/src/Compilation.zig index a474d1955a..c9f9c65e65 100644 --- a/src/Compilation.zig +++ b/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 diff --git a/src/InternPool.zig b/src/InternPool.zig index 4dba928d18..ab5de7c360 100644 --- a/src/InternPool.zig +++ b/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); } diff --git a/src/Zcu.zig b/src/Zcu.zig index a5de69fb62..91b60c6108 100644 --- a/src/Zcu.zig +++ b/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]; } diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index 03d9b08261..7fa9f89d40 100644 --- a/src/Zcu/PerThread.zig +++ b/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,