Skip to content

Drain an autorelease pool on the export worker threads - #297

Open
l7aromeo wants to merge 1 commit into
samizdatco:mainfrom
l7aromeo:fix/worker-autorelease-pool
Open

Drain an autorelease pool on the export worker threads#297
l7aromeo wants to merge 1 commit into
samizdatco:mainfrom
l7aromeo:fix/worker-autorelease-pool

Conversation

@l7aromeo

@l7aromeo l7aromeo commented Aug 8, 2026

Copy link
Copy Markdown

Drain an autorelease pool on the export worker threads

toBuffer and saveAs hand their work to rayon::spawn_fifo. On macOS that work
allocates autoreleased Objective-C objects through Metal, and a rayon worker has no
autorelease pool of its own — so those objects are never drained and accumulate for the
life of the process.

The main thread doesn't have this problem because Node's event loop drains a pool each
tick, which is why the synchronous export methods are unaffected.

Evidence

heap on a process running toBuffer in a loop, sampled 60 seconds apart. Exactly one
class grows:

COUNT   BYTES     CLASS_NAME                  BINARY
2987    477920    AGXG16XFamilyBlitContext    AGXMetalG16X     <- t=25s
9913   1586080    AGXG16XFamilyBlitContext    AGXMetalG16X     <- t=85s

Everything else is flat. Meanwhile the malloc heap total stayed at ~17 MB while RSS
reached 432 MB, which is what you'd expect from allocations owned by the Metal driver
rather than by the allocator — and is why this doesn't show up under leaks or under a
replacement allocator.

MetalEngine::with_context already wraps its body in autoreleasepool, but the export
work in Page::encoded_assurface.direct_context(), make_non_texture_image, the
encoder calls — runs outside it.

The surcharge is Metal-only

If async export leaked for some general reason, it would leak on Vulkan too. It doesn't.
Same package, same version, no patch applied anywhere — only the backend differs:

3.0.8, unpatched sync async async surcharge
Metal (macOS, M4 Pro) 25.0 101.9 +77
Vulkan (Linux x64, GTX 1050 Ti) 26.0 17.3 none
CPU (same Linux box) 25.1 26.0 none
CPU (Linux aarch64) 17.1 15.4 none

The ~25 KiB/render present in every row is the unrelated full-canvas-fill leak (#296),
which is backend-independent. The surcharge on top of it appears only under Metal.

The same comparison on a fork that doesn't carry #296's leak isolates it further: 80.4
KiB/render on Metal versus −0.2 on Vulkan, from a flat baseline.

Vulkan was exercised on one Linux machine with NVIDIA and Intel ICDs, so this doesn't
prove every Vulkan driver is clean — only that the async path costs nothing extra there
while it costs ~77–82 KiB/render under Metal.

The fix

A helper in src/gpu/mod.rs, gated on the metal feature, plus a wrap at the two
spawn_fifo sites in src/canvas.rs:

#[cfg(feature = "metal")]
pub fn autorelease<T>(f:impl FnOnce() -> T) -> T { objc::rc::autoreleasepool(f) }

#[cfg(not(feature = "metal"))]
pub fn autorelease<T>(f:impl FnOnce() -> T) -> T { f() }

Gating on metal rather than on the target OS is deliberate on two counts: objc is an
optional dependency pulled in only by that feature, and the Metal engine is the only
thing in the crate that produces autoreleased objects, so the two conditions coincide.
On every other configuration this expands to the original expression, so non-Metal
builds are unchanged.

Measurements

Second-half RSS slope over 2000 renders, KiB per render:

main this PR
toBufferSync, CPU 25.5 25.1
toBuffer, CPU 26.2 26.0
toBufferSync, GPU 25.0 25.3
toBuffer, GPU 101.9 25.4

This removes the entire GPU-async surcharge (~76 KiB/render) and touches nothing else.
The ~25 KiB/render floor that remains in every cell is a separate, unrelated leak — the
full-canvas fill optimization never firing — which I've opened separately as #296. The two
are independent and compose; with both applied all four cells sit at roughly −1
KiB/render.

Possible relation to #145

#145 reported async toBuffer leaking, was bisected to 68bef1b, and was closed
without a root cause. That commit only changes how Vulkan support is detected, which
flips the default rendering engine on some machines — it doesn't touch any export path.
That would explain why the bisect looked inexplicable: it changed which renderer users
landed on by default rather than introducing a leak.

I can't claim this fully explains #145 — the reporter saw sync-with-GPU as clean whereas
3.0.8 leaks ~25 KiB/render there, and that was a very different codebase — but it does
account for the async-specific component they couldn't pin down.

Testing

  • npm test — 141/141 pass.
  • cargo clippy — no new warnings on the changed lines.
  • cargo check --no-default-features — compiles, confirming the non-Metal path.
  • Output is byte-identical to main. Draining a pool per export could in principle
    invalidate the Image that PageCache holds across calls, so I hashed the output of
    repeated exports on the same canvas — toBuffer('png') followed by toBuffer('jpg'),
    the same format twice, drawing more between exports (the partial-replay path with a
    stale cache_depth), interleaved sync/async, and 100 consecutive exports. All hashes
    match main on both CPU and GPU. Skia holds those textures via sk_sp, so the pool
    pop only balances the autorelease, not the strong reference.

Caveats

  • Measured on macOS 26.6 / Apple M4 Pro / Node v26.4.0. This needs a real Metal device
    to reproduce, so CI won't exercise it — the runners have no GPU and the suite has no
    memory assertions.
  • Verified through toBuffer. The same change is applied to the saveAs /
    write_sequence spawn site, which I did not separately measure.
  • Vulkan needs no pool, so this change is a no-op there by construction — it compiles to
    the original expression. Measured on Linux with a Vulkan-backed GPU to confirm that
    backend has no comparable per-thread issue of its own (table above); that was one
    machine, not a survey of drivers.

Reproducer

// node --expose-gc repro.js   (requires a Metal-capable machine)
const { Canvas } = require('skia-canvas')

async function main() {
  const canvas = new Canvas(1200, 900)
  canvas.gpu = true
  const ctx = canvas.getContext('2d')
  const samples = []

  for (let i = 0; i < 2000; i++) {
    ctx.fillStyle = '#DFCCAE'
    ctx.fillRect(0, 0, 1200, 900)
    ctx.fillStyle = '#2E1F22'
    ctx.font = 'bold 28px sans-serif'
    ctx.fillText(`frame ${i}`, 24, 48)
    ctx.font = '14px sans-serif'
    for (let r = 0; r < 6; r++) {
      for (let c = 0; c < 5; c++) {
        const x = 24 + c * 232, y = 80 + r * 130
        ctx.strokeStyle = '#745557'
        ctx.strokeRect(x, y, 220, 120)
        ctx.fillText(`${i}-${r}-${c}`, x + 8, y + 20)
      }
    }
    await canvas.toBuffer('png')        // toBufferSync here is flat; this is not

    if (i % 100 === 0) {
      global.gc(); global.gc()
      samples.push({ i, rss: process.memoryUsage().rss / 1048576 })
    }
  }

  const mid = samples.length >> 1
  const tail = samples.slice(mid)
  const slope = (tail.at(-1).rss - tail[0].rss) * 1024 / (tail.at(-1).i - tail[0].i)
  console.log(`${slope.toFixed(1)} KiB/render`)
}

main()

On this machine that prints 107.2 KiB/render on main and 27.5 KiB/render with the
patch — the residue being the separate full-canvas-fill leak, which #296 takes to
roughly zero.

Run under heap <pid> to watch AGXG16XFamilyBlitContext climb on main and hold
steady with this applied.

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