Skip to content

Commit 55044b4

Browse files
claudeclaude
committed
revert(prefix-source): drop buffer() override — CodSpeed regressed -77%
CodSpeed (PR #211) flagged: prefix-source: buffer() 3.6 ms -> 16 ms (-77.46%) prefix-source: size() 3.6 ms -> 16 ms (-77.44%) (size() falls back to this.buffer().length, so it tracks buffer()) Why local wall-clock disagreed with CI: my JS-side splice loop (allocUnsafe + per-line indexOf + Buffer.copy) does ~1000 JS<->C++ boundary crossings on the es6-promise.js fixture. Modern CPUs amortize that with branch prediction and cache, so wall-clock looked +17%. CodSpeed measures instruction count via simulation, which counts every boundary crossing — and V8's regex.replace + Buffer.from(string, "utf8") is two big native calls that beat the loop on that metric by a wide margin. Caching the result was off the table from the earlier audit (_source can be mutable). With both single-pass copy AND caching ruled out, there's no path to a faster buffer() in pure JS, so the inherited Source.buffer() default stays. buffers() override stays — it's a new benchmark (no base to regress against) and matches the changeset description "more effective buffers for PrefixSource". Consumers that can accept Buffer[] (writev, fs.createWriteStream) skip the intermediate concatenated buffer; consumers that want a single buffer use buffer() and pay the same as before. The mutable-child regression test still passes. https://claude.ai/code/session_01EHhGq9PRFRGefVtwwasCqZ
1 parent 683c2b7 commit 55044b4

1 file changed

Lines changed: 10 additions & 49 deletions

File tree

‎lib/PrefixSource.js‎

Lines changed: 10 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -62,55 +62,16 @@ class PrefixSource extends Source {
6262
return prefix + node.replace(REPLACE_REGEX, `\n${prefix}`);
6363
}
6464

65-
/**
66-
* Build the prefixed buffer by walking the underlying source's buffer
67-
* directly — skips both the regex replace and the intermediate
68-
* prefixed-string allocation that the default `Source.buffer()` path
69-
* does via `this.source()` + `Buffer.from`. Single linear pass: alloc
70-
* a worst-case-sized scratch with `Buffer.allocUnsafe`, splice the
71-
* prefix in after each `\n` that has more content following, then
72-
* return a tight result.
73-
*
74-
* The worst case is `prefixLen + contentLen + prefixLen * contentLen`
75-
* (every byte is a newline followed by content). For typical short
76-
* prefixes (`"\t"`, `" "`) that's bounded by ~2x the content size and
77-
* we keep the allocUnsafe via `subarray` (zero-copy). For long
78-
* prefixes the ratio is unbounded, so when the over-allocation wastes
79-
* more than 50% we copy into a tight Buffer so the extra capacity
80-
* gets GC'd promptly. No caching — `_source` may be mutable
81-
* (ReplaceSource etc.) and we must reflect changes on each call.
82-
* @returns {Buffer} buffer
83-
*/
84-
buffer() {
85-
const prefix = this._prefix;
86-
if (prefix.length === 0) return this._source.buffer();
87-
const content = this._source.buffer();
88-
const contentLen = content.length;
89-
if (contentLen === 0) return Buffer.from(prefix, "utf8");
90-
const prefixBuffer = Buffer.from(prefix, "utf8");
91-
const prefixLen = prefixBuffer.length;
92-
const worst = prefixLen + contentLen + prefixLen * contentLen;
93-
const result = Buffer.allocUnsafe(worst);
94-
let writePos = prefixBuffer.copy(result, 0);
95-
let readPos = 0;
96-
while (readPos < contentLen) {
97-
const nl = content.indexOf(0x0a, readPos);
98-
const end = nl === -1 ? contentLen : nl + 1;
99-
writePos += content.copy(result, writePos, readPos, end);
100-
// Match the regex `/\n(?=.|\s)/g` — only re-emit the prefix when
101-
// at least one more byte follows the newline.
102-
if (nl !== -1 && nl + 1 < contentLen) {
103-
writePos += prefixBuffer.copy(result, writePos);
104-
}
105-
readPos = end;
106-
}
107-
if (writePos * 2 < worst) {
108-
const tight = Buffer.allocUnsafe(writePos);
109-
result.copy(tight, 0, 0, writePos);
110-
return tight;
111-
}
112-
return result.subarray(0, writePos);
113-
}
65+
// Note: we deliberately don't override buffer() / size() here. The
66+
// inherited Source.buffer() (`Buffer.from(this.source(), "utf8")`) is
67+
// hard to beat — V8's regex.replace + utf8 encode is two big native
68+
// calls, while a JS-side splice loop costs many JS↔C++ boundary
69+
// crossings (CodSpeed measures instruction count, where the JS
70+
// version regressed ~77%). Caching is also off the table because
71+
// `_source` can be mutable (ReplaceSource etc.). buffers() below is
72+
// the only override worth keeping; it gives consumers that can
73+
// accept multiple buffers (writev, fs.createWriteStream) the chance
74+
// to skip the intermediate concatenated buffer.
11475

11576
/**
11677
* Returns a `Buffer[]` that concatenates to the prefixed source. Each

0 commit comments

Comments
 (0)