Improve resize performance by switching to AutoArrayHashMapUnmanaged

I noticed that the HashMap iterator showed up prominently in Instruments when quickly
resizing Ghostty.

I think this is related to the [tombstone issue](https://github.com/ziglang/zig/issues/17851),
where the `next()` function has to skip unused meta-nodes.

In that same issue, Andrew is suggesting that the non-array hashmap might get deleted from the
standard library.

After switching to `AutoArrayHashMapUnmanaged`, iteration barely shows up anymore.

Deletion from the pin list should also be fast as swapRemove is used (order does not need to be preserved).

Question is if insertion performance is negatively affected, though I'm not seeing anything obvious.
Still, checking this PR for any perf regressions might be a good idea.

If this pans out, there are more places where this switch might be beneficial.
This commit is contained in:
cryptocode
2024-08-10 23:54:04 +02:00
parent edea928117
commit 2e88ff1d05

View File

@ -49,7 +49,7 @@ const PagePool = std.heap.MemoryPoolAligned(
/// List of pins, known as "tracked" pins. These are pins that are kept /// List of pins, known as "tracked" pins. These are pins that are kept
/// up to date automatically through page-modifying operations. /// up to date automatically through page-modifying operations.
const PinSet = std.AutoHashMapUnmanaged(*Pin, void); const PinSet = std.AutoArrayHashMapUnmanaged(*Pin, void);
const PinPool = std.heap.MemoryPool(Pin); const PinPool = std.heap.MemoryPool(Pin);
/// The pool of memory used for a pagelist. This can be shared between /// The pool of memory used for a pagelist. This can be shared between
@ -433,9 +433,8 @@ pub fn clone(
// Updating tracked pins is easy, we just change the page // Updating tracked pins is easy, we just change the page
// pointer but all offsets remain the same. // pointer but all offsets remain the same.
if (opts.tracked_pins) |remap| { if (opts.tracked_pins) |remap| {
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != chunk.page) continue; if (p.page != chunk.page) continue;
const new_p = try pool.pins.create(); const new_p = try pool.pins.create();
new_p.* = p.*; new_p.* = p.*;
@ -458,9 +457,8 @@ pub fn clone(
// Updating tracked pins for the pins that are in the shortened chunk. // Updating tracked pins for the pins that are in the shortened chunk.
if (opts.tracked_pins) |remap| { if (opts.tracked_pins) |remap| {
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != chunk.page or if (p.page != chunk.page or
p.y >= chunk.end) continue; p.y >= chunk.end) continue;
const new_p = try pool.pins.create(); const new_p = try pool.pins.create();
@ -502,9 +500,8 @@ pub fn clone(
// Updating tracked pins // Updating tracked pins
if (opts.tracked_pins) |remap| { if (opts.tracked_pins) |remap| {
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != chunk.page or if (p.page != chunk.page or
p.y < chunk.start or p.y < chunk.start or
p.y >= chunk.end) continue; p.y >= chunk.end) continue;
@ -810,9 +807,8 @@ const ReflowCursor = struct {
// Handle tracked pin adjustments. // Handle tracked pin adjustments.
{ {
var it = list.tracked_pins.keyIterator(); const pin_keys = list.tracked_pins.keys();
while (it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (&p.page.data != src_page or if (&p.page.data != src_page or
p.y != src_y) continue; p.y != src_y) continue;
@ -861,9 +857,8 @@ const ReflowCursor = struct {
// Move any tracked pins from the source. // Move any tracked pins from the source.
{ {
var it = list.tracked_pins.keyIterator(); const pin_keys = list.tracked_pins.keys();
while (it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (&p.page.data != src_page or if (&p.page.data != src_page or
p.y != src_y or p.y != src_y or
p.x != x) continue; p.x != x) continue;
@ -1276,9 +1271,8 @@ fn resizeWithoutReflow(self: *PageList, opts: Resize) !void {
// Update all our tracked pins. If they have an X // Update all our tracked pins. If they have an X
// beyond the edge, clamp it. // beyond the edge, clamp it.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.x >= cols) p.x = cols - 1; if (p.x >= cols) p.x = cols - 1;
} }
@ -1495,9 +1489,8 @@ fn resizeWithoutReflowGrowCols(
self.pages.insertBefore(chunk.page, new_page); self.pages.insertBefore(chunk.page, new_page);
// Update our tracked pins that pointed to this previous page. // Update our tracked pins that pointed to this previous page.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != chunk.page or if (p.page != chunk.page or
p.y < y_start or p.y < y_start or
p.y >= y_end) continue; p.y >= y_end) continue;
@ -1561,9 +1554,8 @@ fn trimTrailingBlankRows(
// If our tracked pins are in this row then we cannot trim it // If our tracked pins are in this row then we cannot trim it
// because it implies some sort of importance. If we trimmed this // because it implies some sort of importance. If we trimmed this
// we'd invalidate this pin, as well. // we'd invalidate this pin, as well.
var tracked_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (tracked_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != row_pin.page or if (p.page != row_pin.page or
p.y != row_pin.y) continue; p.y != row_pin.y) continue;
return trimmed; return trimmed;
@ -1793,9 +1785,8 @@ pub fn grow(self: *PageList) !?*List.Node {
// Update any tracked pins that point to this page to point to the // Update any tracked pins that point to this page to point to the
// new first page to the top-left. // new first page to the top-left.
var it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != first) continue; if (p.page != first) continue;
p.page = self.pages.first.?; p.page = self.pages.first.?;
p.y = 0; p.y = 0;
@ -1892,9 +1883,8 @@ pub fn adjustCapacity(
try new_page.data.cloneFrom(&page.data, 0, page.data.size.rows); try new_page.data.cloneFrom(&page.data, 0, page.data.size.rows);
// Fix up all our tracked pins to point to the new page. // Fix up all our tracked pins to point to the new page.
var it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != page) continue; if (p.page != page) continue;
p.page = new_page; p.page = new_page;
} }
@ -2013,9 +2003,8 @@ pub fn eraseRow(
// We adjust the tracked pins in this page, moving up any that were below // We adjust the tracked pins in this page, moving up any that were below
// the removed row. // the removed row.
{ {
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page == page and p.y > pn.y) p.y -= 1; if (p.page == page and p.y > pn.y) p.y -= 1;
} }
} }
@ -2064,9 +2053,8 @@ pub fn eraseRow(
// Our tracked pins for this page need to be updated. // Our tracked pins for this page need to be updated.
// If the pin is in row 0 that means the corresponding row has // If the pin is in row 0 that means the corresponding row has
// been moved to the previous page. Otherwise, move it up by 1. // been moved to the previous page. Otherwise, move it up by 1.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != page) continue; if (p.page != page) continue;
if (p.y == 0) { if (p.y == 0) {
p.page = page.prev.?; p.page = page.prev.?;
@ -2114,9 +2102,8 @@ pub fn eraseRowBounded(
dirty.setRangeValue(.{ .start = pn.y, .end = pn.y + limit }, true); dirty.setRangeValue(.{ .start = pn.y, .end = pn.y + limit }, true);
// Update pins in the shifted region. // Update pins in the shifted region.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page == page and if (p.page == page and
p.y >= pn.y and p.y >= pn.y and
p.y <= pn.y + limit) p.y <= pn.y + limit)
@ -2147,9 +2134,8 @@ pub fn eraseRowBounded(
// Update tracked pins. // Update tracked pins.
{ {
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page == page and p.y >= pn.y) { if (p.page == page and p.y >= pn.y) {
if (p.y == 0) { if (p.y == 0) {
p.x = 0; p.x = 0;
@ -2187,9 +2173,8 @@ pub fn eraseRowBounded(
dirty.setRangeValue(.{ .start = 0, .end = shifted_limit }, true); dirty.setRangeValue(.{ .start = 0, .end = shifted_limit }, true);
// Update pins in the shifted region. // Update pins in the shifted region.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != page or p.y > shifted_limit) continue; if (p.page != page or p.y > shifted_limit) continue;
if (p.y == 0) { if (p.y == 0) {
p.page = page.prev.?; p.page = page.prev.?;
@ -2212,9 +2197,8 @@ pub fn eraseRowBounded(
shifted += page.data.size.rows; shifted += page.data.size.rows;
// Update tracked pins. // Update tracked pins.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != page) continue; if (p.page != page) continue;
if (p.y == 0) { if (p.y == 0) {
p.page = page.prev.?; p.page = page.prev.?;
@ -2298,9 +2282,8 @@ pub fn eraseRows(
// Update any tracked pins to shift their y. If it was in the erased // Update any tracked pins to shift their y. If it was in the erased
// row then we move it to the top of this page. // row then we move it to the top of this page.
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != chunk.page) continue; if (p.page != chunk.page) continue;
if (p.y >= chunk.end) { if (p.y >= chunk.end) {
p.y -= chunk.end; p.y -= chunk.end;
@ -2357,9 +2340,8 @@ fn erasePage(self: *PageList, page: *List.Node) void {
assert(page.next != null or page.prev != null); assert(page.next != null or page.prev != null);
// Update any tracked pins to move to the next page. // Update any tracked pins to move to the next page.
var it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != page) continue; if (p.page != page) continue;
p.page = page.next orelse page.prev orelse unreachable; p.page = page.next orelse page.prev orelse unreachable;
p.y = 0; p.y = 0;
@ -2401,7 +2383,7 @@ pub fn trackPin(self: *PageList, p: Pin) !*Pin {
/// Untrack a previously tracked pin. This will deallocate the pin. /// Untrack a previously tracked pin. This will deallocate the pin.
pub fn untrackPin(self: *PageList, p: *Pin) void { pub fn untrackPin(self: *PageList, p: *Pin) void {
assert(p != self.viewport_pin); assert(p != self.viewport_pin);
if (self.tracked_pins.remove(p)) { if (self.tracked_pins.swapRemove(p)) {
self.pool.pins.destroy(p); self.pool.pins.destroy(p);
} }
} }
@ -2644,9 +2626,8 @@ pub fn diagram(self: *const PageList, writer: anytype) !void {
// don't wanna bother making this function allocating. // don't wanna bother making this function allocating.
var pin_buf: [16]*Pin = undefined; var pin_buf: [16]*Pin = undefined;
var pin_count: usize = 0; var pin_count: usize = 0;
var pin_it = self.tracked_pins.keyIterator(); const pin_keys = self.tracked_pins.keys();
while (pin_it.next()) |p_ptr| { for (pin_keys) |p| {
const p = p_ptr.*;
if (p.page != chunk.page) continue; if (p.page != chunk.page) continue;
if (p.y != y) continue; if (p.y != y) continue;
pin_buf[pin_count] = p; pin_buf[pin_count] = p;