Pair of fixes in meta alloc.

1 files changed, 22 insertions, 11 deletions

diff --git a/src/zisp/gc/meta_alloc.zig b/src/zisp/gc/meta_alloc.zig
index da9074a..d3b6be1 100644
--- a/src/zisp/gc/meta_alloc.zig
+++ b/src/zisp/gc/meta_alloc.zig
@@ -210,13 +210,12 @@ fn alloc_size_class(sci: u8, size: u32) []u8 {
         return ptr[0..size];
     }
 
-    const info = &slab_infos[sci];
-    const res_n = reserveCountForSizeClassIndex(sci);
-
     // Try to transfer a chunk from the global free-list.
+    const info = &slab_infos[sci];
     if (alloc_from_fl(slab, info, tl)) |p| return p[0..size];
 
-    return alloc_fresh(slab, size, res_n * size, info, tl);
+    const res_n = reserveCountForSizeClassIndex(sci);
+    return alloc_fresh(slab, size, info, tl, res_n);
 }
 
 fn alloc_from_fl(
@@ -230,7 +229,9 @@ fn alloc_from_fl(
         const ptr = slab + fl_head.idx * 8;
         const arr: [*]u32 = @ptrCast(@alignCast(ptr));
 
+        // Load this atomically since it's still globally accessible memory.
         const next_head = @atomicLoad(u32, &arr[0], .unordered);
+
         if (@cmpxchgWeak(
             FlHead,
             &info.free_list,
@@ -244,7 +245,9 @@ fn alloc_from_fl(
             continue;
         }
 
+        // Now detached from global; no need for atomics anymore.
         tl.fc_count = arr[1];
+
         // We can safely copy more than needed since we set fc_count; this way
         // it should compile neatly into SIMD instructions.
         for (0..RESERVE_MAX / 2) |i| tl.fc[i] = arr[4 + i];
@@ -257,12 +260,13 @@ fn alloc_from_fl(
 fn alloc_fresh(
     slab: [*]u8,
     size: u32,
-    bump: u32,
     info: *SlabInfo,
     tl: *TlSlabInfo,
+    res_n: u32,
 ) []u8 {
     // Have to use fresh memory from the top of the slab; bump it by a chunk,
     // atomically, to reserve memory for this thread.
+    const bump = res_n * size / 8;
     const old_wm = @atomicRmw(u32, &info.watermark, .Add, bump, .monotonic);
     const new_wm = old_wm + bump;
 
@@ -273,14 +277,14 @@ fn alloc_fresh(
     }
 
     const ptr = slab + old_wm * 8;
-    tl.wm_lo = old_wm + size;
+    tl.wm_lo = old_wm + size / 8;
     tl.wm_hi = new_wm;
     return ptr[0..size];
 }
 
 /// Free memory that was returned by alloc().
 pub fn free(slot: []u8) void {
-    // Can only happen if the slice doesn't originate from our alloc().
+    std.debug.assert(@popCount(slot.len) == 1);
     std.debug.assert(slot.len >= SIZES[0]);
 
     const sci = getSizeClassIndex(slot.len);
@@ -301,12 +305,14 @@ fn free_size_class(sci: u8, slot: []u8) void {
 
     const res_n = reserveCountForSizeClassIndex(sci);
 
+    // Still some space in the local free slot cache.
     if (tl.fc_count != res_n) {
         tl.fc[tl.fc_count] = idx;
         tl.fc_count += 1;
         return;
     }
 
+    // Local cache full; need to flush some to global free-list.
     const info = &slab_infos[sci];
     free_into_fl(info, tl, slot, idx, res_n / 2);
 }
@@ -316,18 +322,23 @@ fn free_into_fl(
     tl: *TlSlabInfo,
     slot: []u8,
     idx: u32,
-    n: u32,
+    num: u32,
 ) void {
     const arr: [*]u32 = @ptrCast(@alignCast(slot.ptr));
 
-    tl.fc_count = n;
+    // Intuitively, this belongs to the end, but it doesn't matter; the rest of
+    // this function is not allowed to fail anyway.
+    tl.fc_count = num;
 
-    arr[1] = n;
+    // No need for atomic stores; we own this memory exclusively right now,
+    // unless another thread calls free() on the same slot which is a bug.
+    arr[1] = num;
     // Safe to copy more than needed; should compile into SIMD.
-    for (0..RESERVE_MAX / 2) |i| arr[4 + i] = tl.fc[n + i];
+    for (0..RESERVE_MAX / 2) |i| arr[4 + i] = tl.fc[num + i];
 
     var fl_head = @atomicLoad(FlHead, &info.free_list, .acquire);
     while (true) {
+        // Still owned exclusively; no need for atomic store.
         arr[0] = fl_head.idx;
         if (@cmpxchgWeak(
             FlHead,