Metal renderer over-allocates cell buffers by 32× (@sizeOf(T) applied twice on grow)

[joshka]

What

Buffer(T).sync() and syncFromArrayLists() in src/renderer/metal/buffer.zig multiply the reallocation size by @sizeOf(T) twice when a GPU buffer grows. req_bytes is already a byte count and size = req_bytes * 2 is the intended byte count, but the allocation passes size * @sizeOf(T):

• buffer.zig#L83-L92 (sync)
• buffer.zig#L140-L149 (syncFromArrayLists)

newBufferWithLength: takes a length in bytes, so every grow allocates @sizeOf(T)× more than intended — 32× for the CellText/CellBg cell buffers (@sizeOf(CellText) == 32). init()/initFill() already do it correctly (len * @sizeOf(T)).

Fix

- @as(c_ulong, @intCast(size * @sizeOf(T))),
+ @as(c_ulong, @intCast(size)),

in both grow paths — size is already the target byte count.

Impact

• Runs on the first rendered frame and every grid growth: the frame buffers start at 1 element by design (generic.zig#L347-L365) and the first updateFrame syncs rows × cols cells (generic.zig#L1571-L1573).
• What it costs (measured, so as not to overstate it): Metal accounts 32× the bytes (currentAllocatedSize grows +195 MB vs +6 MB for a 500×200 grid), but the extra pages are demand-zero and never touched, so process phys_footprint and RSS are unchanged (+3 MB in both cases). This is not a resident-memory regression a user would see in Activity Monitor. The real consequences are: (1) inflated currentAllocatedSize / 32× virtual reservation, and (2) newBufferWithLength: requesting 32× the bytes, which risks failing (→ the existing error.MetalFailed path) for large grids that would otherwise fit. Plus it's simply wrong.
• Not a security issue — it over-allocates (never under), so there's no OOB write or info leak.
• Distinct from the buffer-sizing work in renderer (generic): reset buffer size on grid resize #10355 / renderer: Metal and OpenGL shrink buffers when they're much smaller #10354, which is about buffers not shrinking — neither touches this multiply.

---

Verifying this is real (not an AI-hallucinated report)

This was surfaced by an AI-assisted Rust port of the terminal, which is an easy way to generate plausible-but-wrong bug reports. So rather than ask you to trust it, here is the evidence to confirm it independently:

• Direct readback. A test against a real MTLDevice grows a buffer of a 32-byte element from 1 element and syncs 128 B of data; buffer.length comes back 8192 (expected 256 = data + 2× headroom) — exactly 256 × @sizeOf(T). With the fix it returns 256.
• Memory, measured both ways. For a single cell buffer grown to a 500×200 grid (100,000 × 32-byte cells), MTLDevice.currentAllocatedSize grows +195 MB on main vs +6 MB with the fix — but process phys_footprint/RSS grow +3 MB either way (the over-allocated tail is never touched). So the allocation really is 32×, but it is a virtual/accounting cost, not resident RAM — included here explicitly so the report isn't overstated.
• Not intentional / not a lost units convention. The line is unchanged since 5229cb93d (Aug 2023), and the identical logic existed inline in Metal.zig before that extraction — there too size = req_bytes * 2 was a byte count and size * @sizeOf(GPUCell) was passed. size was never an element count that would make the multiply correct.
• Reproduction branch: joshka/ghostty@repro/metal-buffer-overalloc, on top of c5a21edfc, three commits — a failing buffer.length test, the one-line fix, and the allocation/phys_footprint measurement. Built and run with Zig 0.15.2. (Includes a small build change to link the Metal framework into zig build test, since the renderer's Zig code is normally linked by the Xcode project and has no device-backed tests today.)

PR access

The one-line fix is ready on the branch above. I'm a first-time contributor and not yet vouched — I have an open vouch request in #13232. Per the contributing docs an unvouched PR would be auto-closed, so I've held off opening one. Happy to open the PR (fix, and optionally the regression test) as soon as that's sorted — just let me know the preferred path.

Disclosure: this report was prepared with AI assistance; the finding, history, and measurements were verified against upstream source at c5a21edfc.

[mitchellh]

This was fixed on main. Thanks.