Merge pull request #1880 from qwerasd205/fix-set

Fix a few RefCountedSet problems

src/inspector/page.zig

@@ -37,7 +37,7 @@ pub fn render(page: *const terminal.Page) void {
         }
         {
             _ = cimgui.c.igTableSetColumnIndex(1);
-            cimgui.c.igText("%d", page.styles.count(page.memory));
+            cimgui.c.igText("%d", page.styles.count());
         }
     }
     {

src/terminal/PageList.zig

@@ -490,6 +490,12 @@ pub fn clone(
             src.* = old_dst;
             dirty.setValue(i, dirty.isSet(i + chunk.start));
         }
+
+        // We need to clear the rows we're about to truncate.
+        for (len..page.data.size.rows) |i| {
+            page.data.clearCells(&rows[i], 0, page.data.size.cols);
+        }
+
         page.data.size.rows = @intCast(len);
         total_rows += len;
 
@@ -1839,7 +1845,7 @@ pub const AdjustCapacity = struct {
     /// Adjust the number of styles in the page. This may be
     /// rounded up if necessary to fit alignment requirements,
     /// but it will never be rounded down.
-    styles: ?u16 = null,
+    styles: ?usize = null,
 
     /// Adjust the number of available grapheme bytes in the page.
     grapheme_bytes: ?usize = null,
@@ -1871,7 +1877,7 @@ pub fn adjustCapacity(
     var cap = page.data.capacity;
 
     if (adjustment.styles) |v| {
-        const aligned = try std.math.ceilPowerOfTwo(u16, v);
+        const aligned = try std.math.ceilPowerOfTwo(usize, v);
         cap.styles = @max(cap.styles, aligned);
     }
     if (adjustment.grapheme_bytes) |v| {
@@ -4759,6 +4765,56 @@ test "PageList clone partial trimmed left" {
     try testing.expectEqual(@as(usize, 40), s2.totalRows());
 }
 
+test "PageList clone partial trimmed left reclaims styles" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s = try init(alloc, 80, 20, null);
+    defer s.deinit();
+    try testing.expectEqual(@as(usize, s.rows), s.totalRows());
+    try s.growRows(30);
+
+    // Style the rows we're trimming
+    {
+        try testing.expect(s.pages.first == s.pages.last);
+        const page = &s.pages.first.?.data;
+
+        const style: stylepkg.Style = .{ .flags = .{ .bold = true } };
+        const style_id = try page.styles.add(page.memory, style);
+
+        var it = s.rowIterator(.left_up, .{ .screen = .{} }, .{ .screen = .{ .y = 9 } });
+        while (it.next()) |p| {
+            const rac = p.rowAndCell();
+            rac.row.styled = true;
+            rac.cell.* = .{
+                .content_tag = .codepoint,
+                .content = .{ .codepoint = 'A' },
+                .style_id = style_id,
+            };
+            page.styles.use(page.memory, style_id);
+        }
+
+        // We're over-counted by 1 because `add` implies `use`.
+        page.styles.release(page.memory, style_id);
+
+        // Expect to have one style
+        try testing.expectEqual(1, page.styles.count());
+    }
+
+    var s2 = try s.clone(.{
+        .top = .{ .screen = .{ .y = 10 } },
+        .memory = .{ .alloc = alloc },
+    });
+    defer s2.deinit();
+    try testing.expectEqual(@as(usize, 40), s2.totalRows());
+
+    {
+        try testing.expect(s2.pages.first == s2.pages.last);
+        const page = &s2.pages.first.?.data;
+        try testing.expectEqual(0, page.styles.count());
+    }
+}
+
 test "PageList clone partial trimmed both" {
     const testing = std.testing;
     const alloc = testing.allocator;

src/terminal/Screen.zig

@@ -1175,7 +1175,7 @@ pub fn setAttribute(self: *Screen, attr: sgr.Attribute) !void {
 pub fn manualStyleUpdate(self: *Screen) !void {
     var page = &self.cursor.page_pin.page.data;
 
-    // std.log.warn("active styles={}", .{page.styles.count(page.memory)});
+    // std.log.warn("active styles={}", .{page.styles.count()});
 
     // Release our previous style if it was not default.
     if (self.cursor.style_id != style.default_id) {
@@ -1201,12 +1201,17 @@ pub fn manualStyleUpdate(self: *Screen) !void {
     const id = page.styles.add(
         page.memory,
         self.cursor.style,
-    ) catch id: {
+    ) catch |err| id: {
-        // Our style map is full. Let's allocate a new
+        // Our style map is full or needs to be rehashed,
-        // page by doubling the size and then try again.
+        // so we allocate a new page, which will rehash,
+        // and double the style capacity for it if it was
+        // full.
         const node = try self.pages.adjustCapacity(
             self.cursor.page_pin.page,
-            .{ .styles = page.capacity.styles * 2 },
+            switch (err) {
+                error.OutOfMemory => .{ .styles = page.capacity.styles * 2 },
+                error.NeedsRehash => .{},
+            },
         );
 
         page = &node.data;
@@ -2388,17 +2393,17 @@ test "Screen cursorCopy style deref" {
 
     var s2 = try Screen.init(alloc, 10, 10, 0);
     defer s2.deinit();
-    const page = s2.cursor.page_pin.page.data;
+    const page = &s2.cursor.page_pin.page.data;
 
     // Bold should create our style
     try s2.setAttribute(.{ .bold = {} });
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
     try testing.expect(s2.cursor.style.flags.bold);
 
     // Copy default style, should release our style
     try s2.cursorCopy(s.cursor);
     try testing.expect(!s2.cursor.style.flags.bold);
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
 
 test "Screen cursorCopy style copy" {
@@ -2411,10 +2416,10 @@ test "Screen cursorCopy style copy" {
 
     var s2 = try Screen.init(alloc, 10, 10, 0);
     defer s2.deinit();
-    const page = s2.cursor.page_pin.page.data;
+    const page = &s2.cursor.page_pin.page.data;
     try s2.cursorCopy(s.cursor);
     try testing.expect(s2.cursor.style.flags.bold);
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 }
 
 test "Screen style basics" {
@@ -2423,19 +2428,19 @@ test "Screen style basics" {
 
     var s = try Screen.init(alloc, 80, 24, 1000);
     defer s.deinit();
-    const page = s.cursor.page_pin.page.data;
+    const page = &s.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 
     // Set a new style
     try s.setAttribute(.{ .bold = {} });
     try testing.expect(s.cursor.style_id != 0);
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
     try testing.expect(s.cursor.style.flags.bold);
 
     // Set another style, we should still only have one since it was unused
     try s.setAttribute(.{ .italic = {} });
     try testing.expect(s.cursor.style_id != 0);
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
     try testing.expect(s.cursor.style.flags.italic);
 }
 
@@ -2445,18 +2450,18 @@ test "Screen style reset to default" {
 
     var s = try Screen.init(alloc, 80, 24, 1000);
     defer s.deinit();
-    const page = s.cursor.page_pin.page.data;
+    const page = &s.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 
     // Set a new style
     try s.setAttribute(.{ .bold = {} });
     try testing.expect(s.cursor.style_id != 0);
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 
     // Reset to default
     try s.setAttribute(.{ .reset_bold = {} });
     try testing.expect(s.cursor.style_id == 0);
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
 
 test "Screen style reset with unset" {
@@ -2465,18 +2470,18 @@ test "Screen style reset with unset" {
 
     var s = try Screen.init(alloc, 80, 24, 1000);
     defer s.deinit();
-    const page = s.cursor.page_pin.page.data;
+    const page = &s.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 
     // Set a new style
     try s.setAttribute(.{ .bold = {} });
     try testing.expect(s.cursor.style_id != 0);
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 
     // Reset to default
     try s.setAttribute(.{ .unset = {} });
     try testing.expect(s.cursor.style_id == 0);
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
 
 test "Screen clearRows active one line" {
@@ -2522,13 +2527,13 @@ test "Screen clearRows active styled line" {
     try s.setAttribute(.{ .unset = {} });
 
     // We should have one style
-    const page = s.cursor.page_pin.page.data;
+    const page = &s.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 
     s.clearRows(.{ .active = .{} }, null, false);
 
     // We should have none because active cleared it
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 
     const str = try s.dumpStringAlloc(alloc, .{ .screen = .{} });
     defer alloc.free(str);

src/terminal/Terminal.zig

@@ -2826,8 +2826,8 @@ test "Terminal: print over wide char with bold" {
     try t.print(0x1F600); // Smiley face
     // verify we have styles in our style map
     {
-        const page = t.screen.cursor.page_pin.page.data;
+        const page = &t.screen.cursor.page_pin.page.data;
-        try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+        try testing.expectEqual(@as(usize, 1), page.styles.count());
     }
 
     // Go back and overwrite with no style
@@ -2837,8 +2837,8 @@ test "Terminal: print over wide char with bold" {
 
     // verify our style is gone
     {
-        const page = t.screen.cursor.page_pin.page.data;
+        const page = &t.screen.cursor.page_pin.page.data;
-        try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+        try testing.expectEqual(@as(usize, 0), page.styles.count());
     }
 
     try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } }));
@@ -2856,8 +2856,8 @@ test "Terminal: print over wide char with bg color" {
     try t.print(0x1F600); // Smiley face
     // verify we have styles in our style map
     {
-        const page = t.screen.cursor.page_pin.page.data;
+        const page = &t.screen.cursor.page_pin.page.data;
-        try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+        try testing.expectEqual(@as(usize, 1), page.styles.count());
     }
 
     // Go back and overwrite with no style
@@ -2867,8 +2867,8 @@ test "Terminal: print over wide char with bg color" {
 
     // verify our style is gone
     {
-        const page = t.screen.cursor.page_pin.page.data;
+        const page = &t.screen.cursor.page_pin.page.data;
-        try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+        try testing.expectEqual(@as(usize, 0), page.styles.count());
     }
 
     try testing.expect(t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } }));
@@ -3322,7 +3322,7 @@ test "Terminal: overwrite multicodepoint grapheme clears grapheme data" {
     try testing.expectEqual(@as(usize, 2), t.screen.cursor.x);
 
     // We should have one cell with graphemes
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
     try testing.expectEqual(@as(usize, 1), page.graphemeCount());
 
     // Move back and overwrite wide
@@ -3362,7 +3362,7 @@ test "Terminal: overwrite multicodepoint grapheme tail clears grapheme data" {
     try testing.expectEqual(@as(usize, 2), t.screen.cursor.x);
 
     // We should have one cell with graphemes
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
     try testing.expectEqual(@as(usize, 1), page.graphemeCount());
 
     // Move back and overwrite wide
@@ -4534,8 +4534,8 @@ test "Terminal: insertLines handles style refs" {
     try t.setAttribute(.{ .unset = {} });
 
     // verify we have styles in our style map
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 
     t.setCursorPos(2, 2);
     t.insertLines(1);
@@ -4547,7 +4547,7 @@ test "Terminal: insertLines handles style refs" {
     }
 
     // verify we have no styles in our style map
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
 
 test "Terminal: insertLines outside of scroll region" {
@@ -5336,14 +5336,14 @@ test "Terminal: eraseChars handles refcounted styles" {
     try t.print('C');
 
     // verify we have styles in our style map
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 
     t.setCursorPos(1, 1);
     t.eraseChars(2);
 
     // verify we have no styles in our style map
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
 
 test "Terminal: eraseChars protected attributes respected with iso" {
@@ -7043,7 +7043,7 @@ test "Terminal: bold style" {
         const cell = list_cell.cell;
         try testing.expectEqual(@as(u21, 'A'), cell.content.codepoint);
         try testing.expect(cell.style_id != 0);
-        const page = t.screen.cursor.page_pin.page.data;
+        const page = &t.screen.cursor.page_pin.page.data;
         try testing.expect(page.styles.refCount(page.memory, t.screen.cursor.style_id) > 1);
     }
 }
@@ -7067,8 +7067,8 @@ test "Terminal: garbage collect overwritten" {
     }
 
     // verify we have no styles in our style map
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 0), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 0), page.styles.count());
 }
 
 test "Terminal: do not garbage collect old styles in use" {
@@ -7089,8 +7089,8 @@ test "Terminal: do not garbage collect old styles in use" {
     }
 
     // verify we have no styles in our style map
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
-    try testing.expectEqual(@as(usize, 1), page.styles.count(page.memory));
+    try testing.expectEqual(@as(usize, 1), page.styles.count());
 }
 
 test "Terminal: print with style marks the row as styled" {
@@ -7425,7 +7425,7 @@ test "Terminal: insertBlanks deleting graphemes" {
     try t.print(0x1F467);
 
     // We should have one cell with graphemes
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
     try testing.expectEqual(@as(usize, 1), page.graphemeCount());
 
     t.setCursorPos(1, 1);
@@ -7461,7 +7461,7 @@ test "Terminal: insertBlanks shift graphemes" {
     try t.print(0x1F467);
 
     // We should have one cell with graphemes
-    const page = t.screen.cursor.page_pin.page.data;
+    const page = &t.screen.cursor.page_pin.page.data;
     try testing.expectEqual(@as(usize, 1), page.graphemeCount());
 
     t.setCursorPos(1, 1);

src/terminal/page.zig

@@ -237,6 +237,7 @@ pub const Page = struct {
         MissingStyle,
         UnmarkedStyleRow,
         MismatchedStyleRef,
+        ZombieStyles,
         InvalidStyleCount,
         InvalidSpacerTailLocation,
         InvalidSpacerHeadLocation,
@@ -429,6 +430,37 @@ pub const Page = struct {
                 }
             }
         }
+
+        // Verify there are no zombie styles, that is, styles in the
+        // set with ref counts > 0, which are not present in the page.
+        {
+            const styles_table = self.styles.table.ptr(self.memory)[0..self.styles.layout.table_cap];
+            const styles_items = self.styles.items.ptr(self.memory)[0..self.styles.layout.cap];
+
+            var zombies: usize = 0;
+
+            for (styles_table) |id| {
+                if (id == 0) continue;
+                const item = styles_items[id];
+                if (item.meta.ref == 0) continue;
+
+                const expected = styles_seen.get(id) orelse 0;
+                if (expected > 0) continue;
+
+                if (item.meta.ref > expected) {
+                    zombies += 1;
+                }
+            }
+
+            // Just 1 zombie style might be the cursor style, so ignore it.
+            if (zombies > 1) {
+                log.warn(
+                    "page integrity violation zombie styles count={}",
+                    .{zombies},
+                );
+                return IntegrityError.ZombieStyles;
+            }
+        }
     }
 
     /// Clone the contents of this page. This will allocate new memory
@@ -468,7 +500,7 @@ pub const Page = struct {
         return result;
     }
 
-    pub const CloneFromError = Allocator.Error || error{OutOfMemory};
+    pub const CloneFromError = Allocator.Error || style.Set.AddError;
 
     /// Clone the contents of another page into this page. The capacities
     /// can be different, but the size of the other page must fit into
@@ -1027,7 +1059,7 @@ pub const Capacity = struct {
     rows: size.CellCountInt,
 
     /// Number of unique styles that can be used on this page.
-    styles: u16 = 16,
+    styles: usize = 16,
 
     /// Number of bytes to allocate for grapheme data.
     grapheme_bytes: usize = grapheme_bytes_default,

src/terminal/ref_counted_set.zig

@@ -13,9 +13,9 @@ const fastmem = @import("../fastmem.zig");
 /// the exact memory requirement of a given capacity by calling `layout`
 /// and checking the total size.
 ///
-/// When the set exceeds capacity, `error.OutOfMemory` is returned from
+/// When the set exceeds capacity, an `OutOfMemory` or `NeedsRehash` error
-/// any memory-using methods. The caller is responsible for determining
+/// is returned from any memory-using methods. The caller is responsible
-/// a path forward.
+/// for determining a path forward.
 ///
 /// This set is reference counted. Each item in the set has an associated
 /// reference count. The caller is responsible for calling release for an
@@ -108,6 +108,9 @@ pub fn RefCountedSet(
         /// The backing store of items
         items: Offset(Item),
 
+        /// The number of living items currently stored in the set.
+        living: Id = 0,
+
         /// The next index to store an item at.
         /// Id 0 is reserved for unused items.
         next_id: Id = 1,
@@ -132,21 +135,22 @@ pub fn RefCountedSet(
         ///
         /// The returned layout `cap` property will be 1 more than the number
         /// of items that the set can actually store, since ID 0 is reserved.
-        pub fn layout(cap: Id) Layout {
+        pub fn layout(cap: usize) Layout {
             // Experimentally, this load factor works quite well.
             const load_factor = 0.8125;
 
-            const table_cap: Id = std.math.ceilPowerOfTwoAssert(Id, cap);
+            assert(cap <= @as(usize, @intCast(std.math.maxInt(Id))) + 1);
-            const table_mask: Id = (@as(Id, 1) << std.math.log2_int(Id, table_cap)) - 1;
+
-            const items_cap: Id = @intFromFloat(load_factor * @as(f64, @floatFromInt(table_cap)));
+            const table_cap: usize = std.math.ceilPowerOfTwoAssert(usize, cap);
+            const items_cap: usize = @intFromFloat(load_factor * @as(f64, @floatFromInt(table_cap)));
+
+            const table_mask: Id = @intCast((@as(usize, 1) << std.math.log2_int(usize, table_cap)) - 1);
 
             const table_start = 0;
-            const table_cap_usize: usize = @intCast(table_cap);
+            const table_end = table_start + table_cap * @sizeOf(Id);
-            const table_end = table_start + table_cap_usize * @sizeOf(Id);
 
             const items_start = std.mem.alignForward(usize, table_end, @alignOf(Item));
-            const items_cap_usize: usize = @intCast(items_cap);
+            const items_end = items_start + items_cap * @sizeOf(Item);
-            const items_end = items_start + items_cap_usize * @sizeOf(Item);
 
             const total_size = items_end;
 
@@ -161,8 +165,8 @@ pub fn RefCountedSet(
         }
 
         pub const Layout = struct {
-            cap: Id,
+            cap: usize,
-            table_cap: Id,
+            table_cap: usize,
             table_mask: Id,
             table_start: usize,
             items_start: usize,
@@ -184,12 +188,23 @@ pub fn RefCountedSet(
             };
         }
 
+        /// Possible errors for `add` and `addWithId`.
+        pub const AddError = error{
+            /// There is not enough memory to add a new item.
+            /// Remove items or grow and reinitialize.
+            OutOfMemory,
+
+            /// The set needs to be rehashed, as there are many dead
+            /// items with lower IDs which are inaccessible for re-use.
+            NeedsRehash,
+        };
+
         /// Add an item to the set if not present and increment its ref count.
         ///
         /// Returns the item's ID.
         ///
         /// If the set has no more room, then an OutOfMemory error is returned.
-        pub fn add(self: *Self, base: anytype, value: T) error{OutOfMemory}!Id {
+        pub fn add(self: *Self, base: anytype, value: T) AddError!Id {
             const items = self.items.ptr(base);
 
             // Trim dead items from the end of the list.
@@ -198,14 +213,34 @@ pub fn RefCountedSet(
                 self.deleteItem(base, self.next_id);
             }
 
-            // If we still don't have an available ID, we're out of memory.
+            // If we still don't have an available ID, we can't continue.
-            if (self.next_id >= self.layout.cap) return error.OutOfMemory;
+            if (self.next_id >= self.layout.cap) {
+                // Arbitrarily chosen, threshold for rehashing.
+                // If less than 90% of currently allocated IDs
+                // correspond to living items, we should rehash.
+                // Otherwise, claim we're out of memory because
+                // we assume that we'll end up running out of
+                // memory or rehashing again very soon if we
+                // rehash with only a few IDs left.
+                const rehash_threshold = 0.9;
+                if (self.living < @as(Id, @intFromFloat(@as(f64, @floatFromInt(self.layout.cap)) * rehash_threshold))) {
+                    return AddError.NeedsRehash;
+                }
+
+                // If we don't have at least 10% dead items then
+                // we claim we're out of memory.
+                return AddError.OutOfMemory;
+            }
 
             const id = self.upsert(base, value, self.next_id);
             items[id].meta.ref += 1;
 
             if (id == self.next_id) self.next_id += 1;
 
+            if (items[id].meta.ref == 1) {
+                self.living += 1;
+            }
+
             return id;
         }
 
@@ -215,7 +250,7 @@ pub fn RefCountedSet(
         /// Returns the item's ID, or null if the provided ID was used.
         ///
         /// If the set has no more room, then an OutOfMemory error is returned.
-        pub fn addWithId(self: *Self, base: anytype, value: T, id: Id) error{OutOfMemory}!?Id {
+        pub fn addWithId(self: *Self, base: anytype, value: T, id: Id) AddError!?Id {
             const items = self.items.ptr(base);
 
             if (id < self.next_id) {
@@ -226,6 +261,8 @@ pub fn RefCountedSet(
 
                     items[added_id].meta.ref += 1;
 
+                    self.living += 1;
+
                     return if (added_id == id) null else added_id;
                 } else if (self.context.eql(value, items[id].value)) {
                     items[id].meta.ref += 1;
@@ -302,6 +339,7 @@ pub fn RefCountedSet(
 
             assert(item.meta.ref > 0);
             item.meta.ref -= 1;
+            if (item.meta.ref == 0) self.living -= 1;
         }
 
         /// Release a specified number of references to an item by its ID.
@@ -316,6 +354,10 @@ pub fn RefCountedSet(
 
             assert(item.meta.ref >= n);
             item.meta.ref -= n;
+
+            if (item.meta.ref == 0) {
+                self.living -= 1;
+            }
         }
 
         /// Get the ref count for an item by its ID.
@@ -329,42 +371,8 @@ pub fn RefCountedSet(
         }
 
         /// Get the current number of non-dead items in the set.
-        ///
+        pub fn count(self: *const Self) usize {
-        /// NOT DESIGNED TO BE USED OUTSIDE OF TESTING, this is a very slow
+            return self.living;
-        /// operation, since it traverses the entire structure to count.
-        ///
-        /// Additionally, because this is a testing method, it does extra
-        /// work to verify the integrity of the structure when called.
-        pub fn count(self: *const Self, base: anytype) usize {
-            const table = self.table.ptr(base);
-            const items = self.items.ptr(base);
-
-            // The number of items accessible through the table.
-            var tb_ct: usize = 0;
-
-            for (table[0..self.layout.table_cap]) |id| {
-                if (id != 0) {
-                    const item = items[id];
-                    if (item.meta.ref > 0) {
-                        tb_ct += 1;
-                    }
-                }
-            }
-
-            // The number of items accessible through the backing store.
-            // The two counts should always match- it shouldn't be possible
-            // to have untracked items in the backing store.
-            var it_ct: usize = 0;
-
-            for (items[0..self.layout.cap]) |it| {
-                if (it.meta.ref > 0) {
-                    it_ct += 1;
-                }
-            }
-
-            assert(tb_ct == it_ct);
-
-            return tb_ct;
         }
 
         /// Delete an item, removing any references from
@@ -390,7 +398,7 @@ pub fn RefCountedSet(
             items[id] = .{};
 
             var p: Id = item.meta.bucket;
-            var n: Id = (p + 1) & self.layout.table_mask;
+            var n: Id = (p +% 1) & self.layout.table_mask;
 
             while (table[n] != 0 and items[table[n]].meta.psl > 0) {
                 items[table[n]].meta.bucket = p;
@@ -399,7 +407,7 @@ pub fn RefCountedSet(
                 self.psl_stats[items[table[n]].meta.psl] += 1;
                 table[p] = table[n];
                 p = n;
-                n = (n + 1) & self.layout.table_mask;
+                n = (p +% 1) & self.layout.table_mask;
             }
 
             while (self.max_psl > 0 and self.psl_stats[self.max_psl] == 0) {
@@ -508,6 +516,7 @@ pub fn RefCountedSet(
                     chosen_id = id;
 
                     held_item.meta.bucket = p;
+                    self.psl_stats[item.meta.psl] -= 1;
                     self.psl_stats[held_item.meta.psl] += 1;
                     self.max_psl = @max(self.max_psl, held_item.meta.psl);