Skip to content

Commit 0932c89

Browse files
fix: amortize ChunkReader buffer growth (#3154)
readChunk reallocated and copied the whole buffer on every network read, so one frame cost O(reads²) — ~435 ms vs ~2 ms for the same 1 MiB at the ~66 B reads a slow client actually produces, all blocking the event loop: bounded per request by bodySizeLimit on the server, unbounded on the client leg where the same reader decodes responses. The reader now tracks its backing store separately from the live view and grows in three tiers, cheapest first: append while the store has tail room, compact the consumed prefix (next() advances past drained frames) when reclaiming it makes room, reallocate at >=2x only when a frame genuinely outgrew the store. Same bytes, same framing, same failure behavior — only the allocation strategy changes (frenzzy's option 1, with the compaction tier keeping long multi-frame streams allocation-stable). The regression test counts allocations rather than milliseconds, as the report suggested: one large frame at 66 B reads must allocate O(log reads) (the quadratic code measures ~995, one per read), with correctness pinned across read granularities including boundaries inside the 12-byte header. Reported by @frenzzy with the measurements and the fix plan. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 30f9387 commit 0932c89

3 files changed

Lines changed: 171 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/web": patch
3+
---
4+
5+
Amortize ChunkReader buffer growth: the framed-stream reader reallocated and copied everything received so far on every network read, making one frame O(reads²) — ~200× the CPU for a payload delivered at slow-client read sizes, on both the server (argument decode) and client (response decode) legs. Growth now appends in place, compacts drained frames, and reallocates at ≥2× only when outgrown (#3154)

packages/web/server-functions/src/shared.ts

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -937,19 +937,48 @@ export function createChunk(data) {
937937
export class ChunkReader {
938938
constructor(stream) {
939939
this.reader = stream.getReader();
940-
this.buffer = new Uint8Array(0);
940+
// `buffer` is the view of not-yet-consumed bytes; `store` is its backing
941+
// allocation, kept separately so growth can be amortized (see readChunk).
942+
this.store = new Uint8Array(0);
943+
this.buffer = this.store;
941944
this.done = false;
942945
}
943946

944947
async readChunk() {
945948
const chunk = await this.reader.read();
946-
if (!chunk.done) {
947-
const newBuffer = new Uint8Array(this.buffer.length + chunk.value.length);
948-
newBuffer.set(this.buffer);
949-
newBuffer.set(chunk.value, this.buffer.length);
950-
this.buffer = newBuffer;
951-
} else {
949+
if (chunk.done) {
952950
this.done = true;
951+
return;
952+
}
953+
// Amortized growth (#3154). Reallocating the whole buffer per network
954+
// read made one frame O(reads²): a 1 MiB argument payload delivered at
955+
// the ~66 B reads a slow client actually produces cost ~200× the CPU of
956+
// the same payload at normal read sizes — all of it blocking the event
957+
// loop, inside bodySizeLimit on the server and unbounded on the client
958+
// leg, where the same reader decodes responses. Three tiers, cheapest
959+
// first: append in place while the store has tail room; compact the
960+
// consumed prefix (next() advances `buffer` past drained frames) when
961+
// reclaiming it makes room; reallocate at ≥2× only when the frame
962+
// genuinely outgrew the store. Same bytes, same framing, same failure
963+
// behavior — only the allocation strategy changes.
964+
const incoming = chunk.value;
965+
const store = this.store;
966+
const start = this.buffer.byteOffset;
967+
const end = start + this.buffer.length;
968+
const needed = this.buffer.length + incoming.length;
969+
if (end + incoming.length <= store.length) {
970+
store.set(incoming, end);
971+
this.buffer = store.subarray(start, end + incoming.length);
972+
} else if (needed <= store.length) {
973+
store.copyWithin(0, start, end);
974+
store.set(incoming, this.buffer.length);
975+
this.buffer = store.subarray(0, needed);
976+
} else {
977+
const grown = new Uint8Array(Math.max(needed, store.length * 2));
978+
grown.set(this.buffer);
979+
grown.set(incoming, this.buffer.length);
980+
this.store = grown;
981+
this.buffer = grown.subarray(0, needed);
953982
}
954983
}
955984

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/**
2+
* ChunkReader growth (#3154). The reader used to reallocate-and-copy the
3+
* whole buffer on EVERY network read, making one frame O(reads²): a 1 MiB
4+
* argument payload delivered at the ~66 B reads a slow client actually
5+
* produces cost ~200× the CPU of the same payload at normal read sizes,
6+
* all of it blocking the event loop — bounded per request by bodySizeLimit
7+
* on the server, unbounded on the client leg where the same reader decodes
8+
* responses. The fix is amortized growth (append in tail room, compact the
9+
* consumed prefix, reallocate at ≥2× only when outgrown); nothing about the
10+
* framing, the bytes, or failure behavior changes.
11+
*
12+
* Timing assertions would be flaky, so the durable shape (suggested in the
13+
* report) counts ALLOCATIONS: length-constructed Uint8Arrays during a
14+
* drive must be O(log reads), which fails cleanly on the quadratic code
15+
* (one per read) and passes on the fix.
16+
*/
17+
import { describe, expect, it } from "vitest";
18+
import { ChunkReader, createChunk } from "../../server-functions/src/shared.js";
19+
20+
function concat(chunks: Uint8Array[]) {
21+
const total = chunks.reduce((size, chunk) => size + chunk.length, 0);
22+
const bytes = new Uint8Array(total);
23+
let offset = 0;
24+
for (const chunk of chunks) {
25+
bytes.set(chunk, offset);
26+
offset += chunk.length;
27+
}
28+
return bytes;
29+
}
30+
31+
/** Streams `bytes` in `readSize`-byte deliveries, like a slow socket. */
32+
function streamOf(bytes: Uint8Array, readSize: number) {
33+
let offset = 0;
34+
return new ReadableStream<Uint8Array>({
35+
pull(controller) {
36+
if (offset >= bytes.length) return controller.close();
37+
controller.enqueue(bytes.subarray(offset, Math.min(offset + readSize, bytes.length)));
38+
offset += readSize;
39+
}
40+
});
41+
}
42+
43+
/**
44+
* Runs `drive` with the Uint8Array global replaced by a counting subclass:
45+
* only length constructions (`new Uint8Array(n)`) count — subarray views
46+
* construct from a buffer and stay free, matching what "an allocation"
47+
* means for the growth strategy under test.
48+
*/
49+
async function countingAllocations<T>(drive: () => Promise<T>) {
50+
const Native = globalThis.Uint8Array;
51+
let allocations = 0;
52+
let allocatedBytes = 0;
53+
const Counting = class extends Native {
54+
constructor(...args: any[]) {
55+
super(...(args as [any]));
56+
if (typeof args[0] === "number") {
57+
allocations++;
58+
allocatedBytes += args[0];
59+
}
60+
}
61+
};
62+
(globalThis as any).Uint8Array = Counting;
63+
try {
64+
const value = await drive();
65+
return { value, allocations, allocatedBytes };
66+
} finally {
67+
globalThis.Uint8Array = Native;
68+
}
69+
}
70+
71+
async function drainAll(reader: InstanceType<typeof ChunkReader>) {
72+
const frames: string[] = [];
73+
await reader.drain((frame: string) => frames.push(frame));
74+
return frames;
75+
}
76+
77+
describe("ChunkReader growth (#3154)", () => {
78+
it("delivers identical frames whatever the read granularity", async () => {
79+
const payloads = ["a".repeat(50_000), "", JSON.stringify({ nested: [1, 2, 3] })];
80+
const bytes = concat(payloads.map(createChunk));
81+
82+
// 7 lands read boundaries INSIDE the 12-byte header; 66 is what a slow
83+
// client's coalesced deliveries actually measure; 16384 is a normal read
84+
for (const readSize of [7, 66, 16_384, bytes.length]) {
85+
const frames = await drainAll(new ChunkReader(streamOf(bytes, readSize)));
86+
expect(frames).toEqual(payloads);
87+
}
88+
});
89+
90+
it("allocates O(log reads) for one large frame, not O(reads)", async () => {
91+
const payload = "x".repeat(64 * 1024);
92+
const bytes = concat([createChunk(payload)]);
93+
const reads = Math.ceil(bytes.length / 66); // ~994
94+
95+
const { value, allocations, allocatedBytes } = await countingAllocations(() =>
96+
drainAll(new ChunkReader(streamOf(bytes, 66)))
97+
);
98+
99+
expect(value).toEqual([payload]);
100+
// quadratic code: one allocation per read (~994) totalling ~32 MiB of
101+
// copies; amortized doubling: ~log2(64 KiB) growths plus constants
102+
expect(allocations).toBeLessThan(40);
103+
expect(allocations).toBeGreaterThan(0);
104+
expect(allocatedBytes).toBeLessThan(bytes.length * 8);
105+
});
106+
107+
it("reaches a steady state across many small frames — drained frames recycle the store", async () => {
108+
// next() consumes a frame by advancing the view; the compaction tier
109+
// reclaims that prefix instead of growing, so a long multi-frame stream
110+
// settles into zero new allocations per frame.
111+
const payloads = Array.from({ length: 200 }, (_, i) => `frame-${i}-` + "y".repeat(300));
112+
const bytes = concat(payloads.map(createChunk));
113+
114+
const { value, allocations } = await countingAllocations(() =>
115+
drainAll(new ChunkReader(streamOf(bytes, 66)))
116+
);
117+
118+
expect(value).toEqual(payloads);
119+
expect(allocations).toBeLessThan(40);
120+
});
121+
122+
it("still refuses a truncated stream loudly", async () => {
123+
const bytes = concat([createChunk("complete"), createChunk("cut short")]);
124+
const truncated = bytes.subarray(0, bytes.length - 5);
125+
126+
const reader = new ChunkReader(streamOf(truncated, 66));
127+
expect((await reader.next()).value).toBe("complete");
128+
await expect(reader.next()).rejects.toThrow("Malformed server function stream.");
129+
});
130+
});

0 commit comments

Comments
 (0)