Skip to content

fix(core): free every pooled, dummy and scratch buffer on executor dispose - #61

Merged
chmjkb merged 6 commits into
mainfrom
@chmjkb/gpu-leak-fixes
Oct 26, 2026
Merged

chmjkb merged 6 commits into
mainfrom
@chmjkb/gpu-leak-fixes

Conversation

@chmjkb

@chmjkb chmjkb commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Closes the dispose leaks #60 found in the executor.

  • BufferPool tracks the buffers it has handed out and not taken back, so dispose() also frees ones still in use: a captured frame's target or a toArray result nobody released. Each buffer is either free or in use, never both, so trim() (from fix(core): trim the buffer pool so varying input lengths stop piling up GPU memory #58) stays as it is and nothing it destroys is kept or destroyed twice. New check: trim then dispose destroys each buffer once.
  • gpuExecutor.dispose() frees its f16/f32 dummies and every scratch buffer, including replaced scratches memoized handles may still bind.
  • dispose() also forgets the handles, dummies and scratches it destroys, so an executor used again after it (as bench/ab.ts does after every case) makes fresh ones instead of binding a destroyed buffer. It also ends a capture: the executor can eval and capture again, and a replay() recorded before dispose() throws instead of binding destroyed buffers. New checks: runs again after dispose and runs again after a captured frame is disposed.

The matching checks in eval.gpu.test.ts, replayCache.gpu.test.ts and zoo/leaks.gpu.test.ts drop those buffers from their expected leftovers.

Still left (next PRs)

  • gatherRows uploads its index list per dispatch and never destroys it.
  • toArray on an eager graph never returns the result buffer to the pool.
  • MiniLM's batch path never destroys its segments upload (MinilmModel.forward) or its selector (embedPooledBatch). Its ids and positions are the gatherRows leak above.
  • Kernels that make their own uniform instead of using cachedUniform: fixed in fix(core): take every kernel uniform from cachedUniform so none outlives its executor #62.

replayCache.dispose() disposing each bucket's executor is not in this PR any more: #58 made the same fix on main.

Introduces a breaking change?

  • Yes
  • No

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Performance (kernel tuning, graph building, anything related to performance)
  • Other (chores, tests, code style improvements etc.)

Testing instructions

  1. pnpm --filter runntime exec playwright install chromium (once per machine)
  2. pnpm --filter runntime test

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

private free = new Map<string, GpuBufferRef[]>();
/** Every buffer this pool made, free or in use, so dispose() frees the ones
* a captured frame or a caller still holds. */
private made: GpuBufferRef[] = [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stack branches from before #58, which added trim() to the pool, so it needs a rebase from #60 up (add/add conflicts in bufferPool.test.ts and eval.gpu.test.ts, content conflicts in bufferPool.ts, eval.ts, replayCache.ts).

One thing to fix during the rebase: made keeps every buffer trim() destroys, so it grows by one set per input length forever. Could the pool track only the buffers in use instead (an inUse set: add on acquire, delete on release), and have dispose() destroy free plus inUse? Then trim() needs no change and each buffer sits in exactly one place. Plus a test: trim then dispose destroys each buffer once.

dispose: () => pool.dispose(),
// Forgets every handle, dummy and scratch it destroys, so a later
// dispatch makes fresh ones instead of binding a destroyed buffer.
dispose: () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A captured executor stays frozen after dispose, so it can neither replay nor eval. Reset frozen here too, or note that a captured executor is done after dispose?

@chmjkb
chmjkb added this pull request to stack #65 October 26, 2026 10:28
@chmjkb
chmjkb force-pushed the @chmjkb/gpu-leak-fixes branch from 3b5f012 to 5cda021 Compare October 26, 2026 10:29
@chmjkb
chmjkb marked this pull request as ready for review October 26, 2026 10:39
@chmjkb chmjkb changed the title fix(core): free every pooled, dummy and scratch buffer on executor and replay cache dispose fix(core): free every pooled, dummy and scratch buffer on executor dispose Oct 26, 2026
Base automatically changed from @chmjkb/gpu-leak-tests to main October 26, 2026 10:50
@chmjkb
chmjkb force-pushed the @chmjkb/gpu-leak-fixes branch 2 times, most recently from 1b64fc3 to fabb69c Compare October 26, 2026 10:52
@chmjkb
chmjkb force-pushed the @chmjkb/gpu-leak-fixes branch from fabb69c to bd6f02b Compare October 26, 2026 10:53
@chmjkb
chmjkb requested a review from xxpopielxx October 26, 2026 10:55
@chmjkb
chmjkb merged commit a8e44c9 into main Oct 26, 2026
2 checks passed
@chmjkb
chmjkb deleted the @chmjkb/gpu-leak-fixes branch October 26, 2026 11:19
@chmjkb chmjkb added bug Something isn't working core Engine: graph ops, dispatch, buffer pool, weight loading labels Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core Engine: graph ops, dispatch, buffer pool, weight loading

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants