Skip to content

Reduce read amplification when opening large models - #1597

Open
Hyungkeun-Park-Nota wants to merge 1 commit into
lutzroeder:mainfrom
Hyungkeun-Park-Nota:perf/stream-window-amplification
Open

Reduce read amplification when opening large models#1597
Hyungkeun-Park-Nota wants to merge 1 commit into
lutzroeder:mainfrom
Hyungkeun-Park-Nota:perf/stream-window-amplification

Conversation

@Hyungkeun-Park-Nota

Copy link
Copy Markdown

Fixes the read amplification described in #1596.

FileStream._fill and the two stream-backed readers always filled a fixed 256MB window on a
cache miss, regardless of how many bytes were requested. A 14-byte signature peek read 256MB
from disk, and archive formats whose entries are scattered across the file re-filled the
window once per entry.

This sizes the window to the request instead, with a 64KB minimum, in
node.FileStream._fill, browser.FileStream._fill, protobuf.StreamReader._fill and
python.StreamReader._fill.

Results

Models with an identical 16-node graph and ~1GB of weights, so the difference is purely
data-path cost:

1GB model open (before -> after) disk read RSS
.pt2 1.83s -> 0.13s 4.25x -> 1.00x 609 -> 76MB
.npz 2.80s -> 0.10s 2.25x -> 0.00x 1415 -> 71MB
.pt (state_dict) 0.64s -> 0.12s 2.25x -> 1.00x 613 -> 74MB
.pt (torchscript) 0.70s -> 0.14s 2.25x -> 1.00x 617 -> 80MB
.safetensors 0.19s -> 0.01s 1.25x -> 1.00x 321 -> 54MB
.gguf 0.19s -> 0.01s 1.25x -> 1.00x 323 -> 55MB
.onnx 2.07s -> 1.81s 2.56x -> 1.25x unchanged

Verification

  • node package.js validate passes (lint, tag:validation, desktop and browser Playwright
    tests).
  • Format coverage. test/models.js additionally passes for one model of each of the 71
    types in models.json.
  • Model output is unchanged. For 12 models (the 7 above, small controls, and
    .tar/.tar.gz/gzip-wrapped variants) a fingerprint over the graph structure plus a
    checksum of every initializer's bytes is identical before and after the change.
  • Differential test of the stream classes. The pre-patch and post-patch classes were
    driven side by side with the same randomized sequence of seek/skip/read/peek
    (deterministic seed), asserting they agree with each other and with the underlying bytes:
    node.FileStream 20,060 read/peek ops plus stream() sub-views, browser.FileStream
    29,737 read/peek ops, all byte-identical. Happy to contribute this as a test if there is a
    place for it.
  • No regression on small files (1MB .pt2 0.15s -> 0.12s, 1MB .onnx 0.05s -> 0.05s).
  • The window size was swept from 16KB to 16MB; 16KB-4MB is flat and above 4MB the
    amplification returns, so 64KB is used as the minimum.

The timings are from the Node/desktop path. browser.FileStream carries the same change and
is covered by the differential test and the browser Playwright test, but I have no browser
timings for it.

@lutzroeder
lutzroeder force-pushed the main branch 26 times, most recently from 19102d1 to 936b22e Compare August 26, 2026 15:45
@lutzroeder

lutzroeder commented Aug 27, 2026

Copy link
Copy Markdown
Owner

@Hyungkeun-Park-Nota this change causes regressions in existing test files:

third_party/test/onnx/Llama-3.2-1B-Instruct-q4f16.onnx.zip

  • main: 5.5s, 7.9GB peak memory
  • branch: 36.2s, 14.2GB peak memory

third_party/test/onnx/gpt2.onnx.zip

  • base: 1.2s, 2.5GB peak memory
  • branch: 6.5s, 12.2GB peak memory

FileStream._fill and the stream-backed readers always filled a fixed
256MB window on a cache miss, regardless of how many bytes were
requested, so a 14-byte signature peek read 256MB from disk and formats
whose entries are scattered across the file re-filled the window once per
entry. Size the window to the request instead, with a 64KB minimum.

zip.InflaterStream restarts inflation from the beginning of the entry
whenever the requested end offset exceeds what is already inflated, so a
reader that advances in small steps re-inflates the whole entry on every
step. Grow the target geometrically, which bounds the total inflate work
to about twice the entry size however the reader steps through it.
@Hyungkeun-Park-Nota
Hyungkeun-Park-Nota force-pushed the perf/stream-window-amplification branch from 517b1ad to c16e38f Compare August 27, 2026 02:35
@Hyungkeun-Park-Nota

Copy link
Copy Markdown
Author

Thanks — reproduced both, and the cause was not the window size itself.

zip.InflaterStream._inflate restarts inflation from the beginning of the entry whenever the
requested end offset exceeds what is already inflated. The 256MB window advanced the
high-water mark in large jumps so this happened a handful of times; a smaller window made it
advance in small steps and turned a sequential scan into repeated full inflations. On
gpt2.onnx.zip, inflateRaw went from 5 calls over 1.49GB to 30 calls over 11.14GB — which
is the peak memory you saw.

Fixed by growing the inflate target geometrically, which bounds the total inflate work to
about twice the entry size however the reader steps through it:

if (size !== undefined && this._buffer !== undefined) {
    size = Math.max(size, this._buffer.length * 2);
}

Both files are now faster than main (time / peak RSS):

main first push now
Llama-3.2-1B-Instruct-q4f16.onnx.zip 18.5s / 5.65GB 183s / 54.5GB 10.8s / 3.92GB
gpt2.onnx.zip 5.4s / 2.08GB 33.8s / 11.1GB 4.6s / 2.09GB

On models with an identical graph and ~1GB of weights, .pt2 goes 2.69s / 1.06GB ->
0.15s / 0.07GB, .npz 1.81s / 1.32GB -> 0.09s / 0.07GB, .gguf 0.24s / 0.31GB ->
0.01s / 0.05GB, .onnx 1.92s / 1.56GB -> 1.65s / 1.18GB. Indexed formats gain most because
opening them only needs the index; ONNX gains least because protobuf has no index and the
graph really does require a full pass.

node package.js validate passes. I also ran an A/B pass over all 1271 models in models.json that download here, comparing
open time, peak RSS and a graph fingerprint on main and on this branch. No fingerprint
differences, no status changes, and nothing slower or using more memory than main.

One thing worth flagging: our numbers for the same file diverged a lot — you measured
36.2s / 14.2GB for Llama-3.2-1B-Instruct-q4f16.onnx.zip where I measured 183s / 54.5GB, so
the effect is very sensitive to the machine. The numbers above are from mine; yours are the
ones that should decide whether this is worth taking.

My original benchmarks only covered uncompressed containers, which is why the inflate path
never appeared in them. They now include the compressed cases.

@lutzroeder
lutzroeder force-pushed the main branch 8 times, most recently from 0708c0b to ca43c4e Compare August 29, 2026 17:25
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.

2 participants