Skip to content

fix: reduce retained geometry memory in large reviews#521

Open
matthew-hre wants to merge 2 commits into
modem-dev:mainfrom
matthew-hre:matthew-hre/geometry-fix
Open

fix: reduce retained geometry memory in large reviews#521
matthew-hre wants to merge 2 commits into
modem-dev:mainfrom
matthew-hre:matthew-hre/geometry-fix

Conversation

@matthew-hre

Copy link
Copy Markdown

Hello! Been a big fan of Hunk recently, I use it basically every day at work -- however, it's been a bit of a memory-hog on my machine, so I thought I'd start doing some digging and hopefully contribute some performance patches! First contribution on this project, let me know if I missed anything.

This PR introduces a fix that reduces memory usage in large reviews by making diff section geometry hold on to less data.

Previously, DiffSectionGeometry was holding on to the full planned row stream for every measured file section -- most review interactions really only need lightweight geometry data: row bounds, hunk bounds, body height, and lookup maps.

Since we already window the rendered rows, keeping full planned rows for the entire changeset means we're still retaining a bunch of row objects, even when most files weren't mounted.

This changes geometry to keep the bounds/navigation data eagerly, but rebuild planned rows lazily, only when copy selection asks for row content.

I also added a focused bench:geometry-memory benchmark so this path is easier to measure next time.

Results

The biggest improvement showed up on the huge synthetic review benchmark:

Metric Main This PR Difference
first-frame heap 228.5MB 175.1MB -53.4MB / -23%
after-navigation heap 324.4MB 248.9MB -75.5MB / -23%
after-navigation RSS 708.8MB 644.3MB -64.4MB / -9%

The resize memory benchmark also improved, which makes sense:

Metric Main This PR Difference
final heap 149.7MB 120.0MB -29.6MB / -20%
heap growth 110.2MB 88.0MB -22.1MB / -20%
peak RSS 481.2MB 453.1MB -28.2MB / -6%
RSS growth 216.6MB 186.8MB -29.8MB / -14%

Little less impressive on the memory.ts benchmark, but still positive:

Metric Main median This PR median Difference
first-frame heap 35.6MB 31.3MB -4.3MB / -12%
after-navigation RSS 481.2MB 465.4MB -15.8MB / -3%

I also ran the new focused geometry benchmark on this branch:

bun run benchmarks/geometry-memory.ts
METRIC geometry_heap_growth_bytes=7469892
METRIC geometry_rss_growth_bytes=5898240
METRIC materialize_planned_rows_ms=6.17
METRIC materialized_planned_rows=10260
METRIC materialized_planned_rows_heap_growth_bytes=0

This should be fairly cut-and-dry. Rendering still builds planned rows for mounted sections, and copy selection still gets planned rows when it needs them. The only difference really is that cached geometry no longer keeps those rows alive for every file by default.

@greptile-apps

greptile-apps Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant