Skip to content

Commit faf48bf

Browse files
ronagaduh95
ronag
authored and
aduh95
committed
buffer: limit UTF-8 fast calls to safe one-byte strings
Replace the allocating UTF-8 fast call with a fast binding for sequential one-byte strings. Its slow fallback only returns a Smi sentinel. JavaScript sends rejected strings to a separate regular binding that can flatten strings and handle lazy deoptimization. Both native callbacks used by the fast API avoid JS-heap allocation. Add GC regression and string-representation coverage, including optimized fast acceptance and rejection checks. Fixes: #66870 Refs: #67134 Assisted-by: Codex Signed-off-by: Robert Nagy <ronagy@icloud.com> PR-URL: #67136 Refs: #66870 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
1 parent d3cf7a8 commit faf48bf

6 files changed

Lines changed: 192 additions & 32 deletions

File tree

‎lib/buffer.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ const {
5656
} = primordials;
5757

5858
const {
59-
byteLengthUtf8,
59+
byteLengthUtf8: byteLengthUtf8Slow,
60+
byteLengthUtf8Fast,
6061
compare: _compare,
6162
compareOffset,
6263
copy: _copy,
@@ -907,6 +908,11 @@ function getEncodingOps(encoding) {
907908
}
908909
}
909910

911+
function byteLengthUtf8(string) {
912+
const length = byteLengthUtf8Fast(string);
913+
return length === -1 ? byteLengthUtf8Slow(string) : length;
914+
}
915+
910916
function byteLength(string, encoding) {
911917
if (typeof string !== 'string') {
912918
if (isArrayBufferView(string) || isAnyArrayBuffer(string)) {

‎src/node_buffer.cc‎

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -826,39 +826,27 @@ void SlowByteLengthUtf8(const FunctionCallbackInfo<Value>& args) {
826826
args.GetReturnValue().Set(static_cast<uint64_t>(str->Utf8LengthV2(isolate)));
827827
}
828828

829-
uint32_t FastByteLengthUtf8(
830-
Local<Value> receiver,
831-
Local<Value> sourceValue,
832-
FastApiCallbackOptions& options) { // NOLINT(runtime/references)
829+
// The fast API's slow fallback must not flatten strings either: it uses the
830+
// same unsupported lazy-deoptimization continuation as the fast callback.
831+
// Return a Smi sentinel so JavaScript can make a separate ordinary API call.
832+
void ByteLengthUtf8FastFallback(const FunctionCallbackInfo<Value>& args) {
833+
args.GetReturnValue().Set(-1);
834+
}
835+
836+
int32_t FastByteLengthUtf8(Local<Value> receiver,
837+
const FastOneByteString& source) {
833838
TRACK_V8_FAST_API_CALL("Buffer::FastByteLengthUtf8");
834-
auto isolate = options.isolate;
835-
HandleScope handleScope(isolate);
836-
CHECK(sourceValue->IsString());
837-
Local<String> sourceStr = sourceValue.As<String>();
838-
839-
if (!sourceStr->IsExternalOneByte()) {
840-
// Below ~512 units, or for one-byte, V8's Utf8LengthV2 is faster.
841-
if (sourceStr->Length() >= 512 && !sourceStr->IsOneByte()) {
842-
String::ValueView view(isolate, sourceStr);
843-
if (!view.is_one_byte()) {
844-
// with_replacement matches Buffer.from's U+FFFD for lone surrogates.
845-
return simdutf::utf8_length_from_utf16_with_replacement(
846-
reinterpret_cast<const char16_t*>(view.data16()),
847-
view.length())
848-
.count;
849-
}
850-
}
851-
return sourceStr->Utf8LengthV2(isolate);
852-
}
853-
auto source = sourceStr->GetExternalOneByteStringResource();
839+
// Even two UTF-8 bytes per Latin-1 code unit fit in a 31-bit Smi.
840+
static_assert(String::kMaxLength <= INT32_MAX / 4);
854841
// For short inputs, the function call overhead to simdutf is maybe
855842
// not worth it, reserve simdutf for long strings.
856-
if (source->length() > 128) {
857-
return simdutf::utf8_length_from_latin1(source->data(), source->length());
843+
if (source.length > 128) {
844+
return static_cast<int32_t>(
845+
simdutf::utf8_length_from_latin1(source.data, source.length));
858846
}
859847

860-
uint32_t length = source->length();
861-
const auto input = reinterpret_cast<const uint8_t*>(source->data());
848+
uint32_t length = source.length;
849+
const auto input = reinterpret_cast<const uint8_t*>(source.data);
862850

863851
uint32_t answer = length;
864852
uint32_t i = 0;
@@ -889,7 +877,7 @@ uint32_t FastByteLengthUtf8(
889877
answer += input[i] >> 7;
890878
}
891879

892-
return answer;
880+
return static_cast<int32_t>(answer);
893881
}
894882

895883
static CFunction fast_byte_length_utf8(CFunction::Make(FastByteLengthUtf8));
@@ -2029,10 +2017,11 @@ void Initialize(Local<Object> target,
20292017

20302018
SetMethod(context, target, "setBufferPrototype", SetBufferPrototype);
20312019

2020+
SetMethodNoSideEffect(context, target, "byteLengthUtf8", SlowByteLengthUtf8);
20322021
SetFastMethodNoSideEffect(context,
20332022
target,
2034-
"byteLengthUtf8",
2035-
SlowByteLengthUtf8,
2023+
"byteLengthUtf8Fast",
2024+
ByteLengthUtf8FastFallback,
20362025
&fast_byte_length_utf8);
20372026
SetFastMethod(context, target, "copy", SlowCopy, &fast_copy);
20382027
SetFastMethodNoSideEffect(context, target, "compare", Compare, &fast_compare);
@@ -2123,6 +2112,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
21232112
registry->Register(SetBufferPrototype);
21242113

21252114
registry->Register(SlowByteLengthUtf8);
2115+
registry->Register(ByteLengthUtf8FastFallback);
21262116
registry->Register(fast_byte_length_utf8);
21272117
registry->Register(SlowCopy);
21282118
registry->Register(fast_copy);

‎test/parallel/test-buffer-bytelength-gc.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Flags: --allow-natives-syntax --gc-global --gc-interval=500
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('assert');
6+
7+
function byteLength(object, string) {
8+
return object.x + Buffer.byteLength(string);
9+
}
10+
11+
function makeDying() {
12+
const object = { x: 1 };
13+
object.dying = 1;
14+
return object;
15+
}
16+
17+
const keep = { x: 2, keep: 1 };
18+
19+
// The optimized caller holds the dying objects' map weakly. A full GC while
20+
// flattening a cons string can invalidate that map and lazily deoptimize it.
21+
// A value-returning fast API call, including its slow fallback, cannot handle
22+
// that deoptimization and aborts instead.
23+
eval('%PrepareFunctionForOptimization(byteLength)');
24+
for (let i = 0; i < 10; i++) {
25+
byteLength(makeDying(), 'ab');
26+
byteLength(keep, 'ab');
27+
}
28+
eval('%OptimizeFunctionOnNextCall(byteLength)');
29+
assert.strictEqual(byteLength(keep, 'ab'), 4);
30+
31+
const big = 'a'.repeat(1 << 20);
32+
for (let i = 0; i < 50; i++) {
33+
const cons = big + String(i) + big;
34+
assert.strictEqual(byteLength(keep, cons), 2 + cons.length);
35+
}

‎test/parallel/test-buffer-bytelength-safe-fast.js‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Flags: --expose-internals --no-warnings --allow-natives-syntax --expose_externalize_string
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('assert');
6+
const { internalBinding } = require('internal/test/binding');
7+
const { byteLengthUtf8Fast } = internalBinding('buffer');
8+
9+
const {
10+
createExternalizableString,
11+
createExternalizableTwoByteString,
12+
externalizeString,
13+
isOneByteString,
14+
} = globalThis;
15+
common.allowGlobals(
16+
createExternalizableString,
17+
createExternalizableTwoByteString,
18+
externalizeString,
19+
isOneByteString,
20+
globalThis.x,
21+
);
22+
23+
const flat = ['', 'hello', '\xe9', '\x00\x01\x7f\x80\xff'];
24+
for (const length of [1, 7, 8, 9, 127, 128, 129, 1024]) {
25+
flat.push(Buffer.alloc(length, 0xe9).toString('latin1'));
26+
}
27+
const latin1 = flat[flat.length - 1];
28+
const externalOneByte = createExternalizableString('external é'.repeat(20));
29+
const externalTwoByte = createExternalizableTwoByteString('external €'.repeat(20));
30+
externalizeString(externalOneByte);
31+
externalizeString(externalTwoByte);
32+
const fallback = [
33+
'x'.repeat(1000) + latin1,
34+
latin1.slice(1, 1000),
35+
'€',
36+
'\ud83d',
37+
'a€😀'.repeat(200),
38+
externalOneByte,
39+
externalTwoByte,
40+
];
41+
42+
function fastLength(string) {
43+
return byteLengthUtf8Fast(string);
44+
}
45+
46+
eval('%PrepareFunctionForOptimization(fastLength)');
47+
for (const string of [...flat, ...fallback]) {
48+
// Before optimization the callback only returns the rejection sentinel.
49+
assert.strictEqual(fastLength(string), -1);
50+
}
51+
eval('%OptimizeFunctionOnNextCall(fastLength)');
52+
for (const string of flat) {
53+
assert.strictEqual(fastLength(string), Buffer.from(string).length);
54+
}
55+
for (const string of fallback) {
56+
assert.strictEqual(fastLength(string), -1);
57+
}
58+
assert.strictEqual(fastLength('hello'), 5);
59+
60+
if (common.isDebug) {
61+
const { getV8FastApiCallCount } = internalBinding('debug');
62+
assert.strictEqual(
63+
getV8FastApiCallCount('Buffer::FastByteLengthUtf8'), flat.length + 1);
64+
}
65+
66+
const { byteLength } = Buffer;
67+
function test() {
68+
for (const string of [...flat, ...fallback]) {
69+
assert.strictEqual(byteLength(string), Buffer.from(string).length);
70+
assert.strictEqual(byteLength(string, 'UTF-8'), Buffer.from(string).length);
71+
assert.strictEqual(byteLength(string, 'unknown'), Buffer.from(string).length);
72+
}
73+
}
74+
eval('%PrepareFunctionForOptimization(byteLength)');
75+
test();
76+
eval('%OptimizeFunctionOnNextCall(byteLength)');
77+
test();

‎test/parallel/test-buffer-bytelength-strings.js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Flags: --allow-natives-syntax --expose_externalize_string
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('assert');
6+
7+
const {
8+
createExternalizableString,
9+
createExternalizableTwoByteString,
10+
externalizeString,
11+
isOneByteString,
12+
} = globalThis;
13+
14+
common.allowGlobals(
15+
createExternalizableString,
16+
createExternalizableTwoByteString,
17+
externalizeString,
18+
isOneByteString,
19+
globalThis.x,
20+
);
21+
22+
const { byteLength } = Buffer;
23+
const latin1 = Buffer.from('aé'.repeat(100), 'latin1').toString('latin1');
24+
const external = createExternalizableString('external é'.repeat(20));
25+
externalizeString(external);
26+
assert.strictEqual(isOneByteString(external), true);
27+
28+
// Check flat, cons, sliced, two-byte and external strings, including lone
29+
// surrogates, before and after optimizing the caller.
30+
const cases = [
31+
'hello',
32+
'é',
33+
latin1,
34+
'x'.repeat(1000) + latin1,
35+
latin1.slice(1, 150),
36+
'€',
37+
'\ud83d',
38+
'a€😀'.repeat(200),
39+
external,
40+
];
41+
42+
function test() {
43+
for (const string of cases) {
44+
assert.strictEqual(byteLength(string), Buffer.from(string).length);
45+
}
46+
}
47+
48+
eval('%PrepareFunctionForOptimization(byteLength)');
49+
test();
50+
eval('%OptimizeFunctionOnNextCall(byteLength)');
51+
test();

‎typings/internalBinding/buffer.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export interface BufferBinding {
55
setBufferPrototype(proto: object): void;
66

77
byteLengthUtf8(str: string): number;
8+
byteLengthUtf8Fast(str: string): number;
89
copy(source: ArrayBufferView, target: ArrayBufferView, targetStart: number, sourceStart: number, toCopy: number): number;
910
compare(a: ArrayBufferView, b: ArrayBufferView): number;
1011
compareOffset(source: ArrayBufferView, target: ArrayBufferView, targetStart?: number, sourceStart?: number, targetEnd?: number, sourceEnd?: number): number;

0 commit comments

Comments
 (0)