Skip to content

Commit 8b7309e

Browse files
TheUltDevclaude
andauthored
bindings-typescript: share the string codec and add an ASCII fast path to writeString (#5783)
# Description of Changes `BinaryWriter.writeString` allocates a fresh `TextEncoder` plus a temporary `Uint8Array` for every string it writes, and `BinaryReader.readString` a fresh `TextDecoder` for every string it reads. Both run once per string column per row on every insert/update/find/scan inside a module, so the per-call allocations dominate the cost of string columns. This PR: - **Writer:** writes pure-ASCII strings straight into the writer buffer, one byte per char, with no intermediate allocation; non-ASCII input falls back to a single shared `TextEncoder`. (`TextEncoder.encodeInto` would be the natural tool, but the module host's `TextEncoder` does not provide it, so the fast path is a plain loop.) - **Reader:** one shared `TextDecoder` instead of one per call. The decode itself is unchanged; a per-char ASCII loop was measured and is slower than a single `decode` at typical field lengths, so the reader keeps it. Output is byte-identical to the previous implementation (tests compare against `TextEncoder` for empty, ASCII, Latin-1, CJK, astral/surrogate pairs, mixed, 10k-char strings, and buffer growth from a 1-byte initial capacity). Measured inside a module on a stock 2.8.2 server (20k inserts, `Date.now()` around the loop): | table | before | after | |---|---|---| | 12 string columns | 255-300 ms | 78-88 ms | | 12 f64 columns (control) | ~42-124 ms | unchanged | | 1 string column holding a 305-byte JSON blob | ~80 ms | ~60 ms | The same bytes through fewer allocations; the f64 control shows the rest of the insert path was never the bottleneck. # API and ABI breaking changes None. Same wire bytes, same public surface. # Expected complexity level and risk 1. Two methods, no behavior change beyond allocation; covered by byte-equality tests. # Testing - [x] `crates/bindings-typescript`: `vitest run` (29 files, 301 tests) including the new `writeString` suite (encodes like `TextEncoder` for empty / ASCII / Latin-1 / CJK / astral / mixed / 10k-char inputs, grows from a 1-byte buffer, round-trips through `readString`, consecutive writes stay contiguous). - [x] `eslint` and `prettier --check` on the touched files. - [x] Exercised end to end by a module publishing to a 2.8.2 server with string, option<string> and mixed-type rows, including non-ASCII round trips through the HTTP call path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) # Rollback safety impact n/a Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent d7237e0 commit 8b7309e

3 files changed

Lines changed: 85 additions & 4 deletions

File tree

crates/bindings-typescript/src/lib/binary_reader.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Shared decoder: `readString` is called once per string column per row.
2+
const textDecoder = new TextDecoder('utf-8');
3+
14
export default class BinaryReader {
25
/**
36
* The DataView used to read values from the binary data.
@@ -196,6 +199,6 @@ export default class BinaryReader {
196199
// A view is safe here: TextDecoder copies the bytes synchronously, so nothing
197200
// retains a reference to the reader's buffer. Avoids readUInt8Array's copy.
198201
const bytes = this.readBytes(length);
199-
return new TextDecoder('utf-8').decode(bytes);
202+
return textDecoder.decode(bytes);
200203
}
201204
}

crates/bindings-typescript/src/lib/binary_writer.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { fromByteArray } from 'base64-js';
22

3+
// Shared encoder: `writeString` is called once per string column per row.
4+
const textEncoder = new TextEncoder();
5+
36
const ArrayBufferPrototypeTransfer =
47
ArrayBuffer.prototype.transfer ??
58
function (this: ArrayBuffer, newByteLength) {
@@ -206,8 +209,26 @@ export default class BinaryWriter {
206209
}
207210

208211
writeString(value: string): void {
209-
const encoder = new TextEncoder();
210-
const encodedString = encoder.encode(value);
211-
this.writeUInt8Array(encodedString);
212+
// Fast path: pure-ASCII strings are written straight into the buffer,
213+
// one byte per char, with no intermediate allocation. Non-ASCII input
214+
// falls back to the shared encoder. This method runs once per string
215+
// column per row on every insert/update, so per-call allocations
216+
// (a fresh TextEncoder and a temporary Uint8Array) dominated its cost.
217+
const len = value.length;
218+
this.expandBuffer(4 + len);
219+
const bytes = new Uint8Array(this.buffer.buffer);
220+
let offset = this.offset + 4;
221+
let i = 0;
222+
for (; i < len; i++) {
223+
const c = value.charCodeAt(i);
224+
if (c >= 0x80) break;
225+
bytes[offset++] = c;
226+
}
227+
if (i === len) {
228+
this.view.setUint32(this.offset, len, true);
229+
this.offset = offset;
230+
return;
231+
}
232+
this.writeUInt8Array(textEncoder.encode(value));
212233
}
213234
}

crates/bindings-typescript/tests/binary_read_write.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,3 +186,60 @@ describe('readUInt8Array buffer ownership', () => {
186186
expect(value).toBe('héllo ☃');
187187
});
188188
});
189+
190+
describe('writeString', () => {
191+
// BSATN layout for `string`: u32 byte length (LE) followed by UTF-8 bytes.
192+
const expected = (value: string): number[] => {
193+
const utf8 = new TextEncoder().encode(value);
194+
return [
195+
utf8.length & 0xff,
196+
(utf8.length >> 8) & 0xff,
197+
(utf8.length >> 16) & 0xff,
198+
(utf8.length >> 24) & 0xff,
199+
...utf8,
200+
];
201+
};
202+
const written = (value: string, initialCapacity = 8): number[] => {
203+
const writer = new BinaryWriter(initialCapacity);
204+
writer.writeString(value);
205+
return [...writer.getBuffer()];
206+
};
207+
208+
test.each([
209+
['empty', ''],
210+
['ascii', 'hello world'],
211+
['ascii at the fast-path boundary', '\x7f'],
212+
['latin-1', 'héllo'],
213+
['cjk', '日本語'],
214+
['astral (surrogate pair)', 'emoji 🎉 mix'],
215+
['ascii prefix then non-ascii', 'ascii then ÿ'],
216+
['long ascii', 'x'.repeat(10_000)],
217+
['long non-ascii', 'ü'.repeat(10_000)],
218+
])('%s encodes like TextEncoder', (_name, value) => {
219+
expect(written(value)).toEqual(expected(value));
220+
});
221+
222+
test('grows the buffer from a tiny initial capacity', () => {
223+
expect(written('hello world', 1)).toEqual(expected('hello world'));
224+
expect(written('héllo', 1)).toEqual(expected('héllo'));
225+
});
226+
227+
test('round-trips through readString', () => {
228+
for (const value of ['', 'plain', 'héllo ☃', '🎉'.repeat(100)]) {
229+
const writer = new BinaryWriter(4);
230+
writer.writeString(value);
231+
expect(new BinaryReader(writer.getBuffer()).readString()).toBe(value);
232+
}
233+
});
234+
235+
test('consecutive writes stay contiguous', () => {
236+
const writer = new BinaryWriter(4);
237+
writer.writeString('ab');
238+
writer.writeString('ç');
239+
writer.writeU8(7);
240+
const reader = new BinaryReader(writer.getBuffer());
241+
expect(reader.readString()).toBe('ab');
242+
expect(reader.readString()).toBe('ç');
243+
expect(reader.readU8()).toBe(7);
244+
});
245+
});

0 commit comments

Comments
 (0)