buffer: limit UTF-8 fast calls to safe one-byte strings

lib/buffer.js

@@ -56,7 +56,8 @@ const {
 } = primordials;
 
 const {
-  byteLengthUtf8,
+  byteLengthUtf8: byteLengthUtf8Slow,
+  byteLengthUtf8Fast,
   compare: _compare,
   compareOffset,
   copy: _copy,
@@ -907,6 +908,11 @@ function getEncodingOps(encoding) {
   }
 }
 
+function byteLengthUtf8(string) {
+  const length = byteLengthUtf8Fast(string);
+  return length === -1 ? byteLengthUtf8Slow(string) : length;
+}
+
 function byteLength(string, encoding) {
   if (typeof string !== 'string') {
     if (isArrayBufferView(string) || isAnyArrayBuffer(string)) {

src/node_buffer.cc

@@ -826,39 +826,27 @@ void SlowByteLengthUtf8(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(static_cast<uint64_t>(str->Utf8LengthV2(isolate)));
 }
 
-uint32_t FastByteLengthUtf8(
-    Local<Value> receiver,
-    Local<Value> sourceValue,
-    FastApiCallbackOptions& options) {  // NOLINT(runtime/references)
+// The fast API's slow fallback must not flatten strings either: it uses the
+// same unsupported lazy-deoptimization continuation as the fast callback.
+// Return a Smi sentinel so JavaScript can make a separate ordinary API call.
+void ByteLengthUtf8FastFallback(const FunctionCallbackInfo<Value>& args) {
+  args.GetReturnValue().Set(-1);
+}
+
+int32_t FastByteLengthUtf8(Local<Value> receiver,
+                           const FastOneByteString& source) {
   TRACK_V8_FAST_API_CALL("Buffer::FastByteLengthUtf8");
-  auto isolate = options.isolate;
-  HandleScope handleScope(isolate);
-  CHECK(sourceValue->IsString());
-  Local<String> sourceStr = sourceValue.As<String>();
-
-  if (!sourceStr->IsExternalOneByte()) {
-    // Below ~512 units, or for one-byte, V8's Utf8LengthV2 is faster.
-    if (sourceStr->Length() >= 512 && !sourceStr->IsOneByte()) {
-      String::ValueView view(isolate, sourceStr);
-      if (!view.is_one_byte()) {
-        // with_replacement matches Buffer.from's U+FFFD for lone surrogates.
-        return simdutf::utf8_length_from_utf16_with_replacement(
-                   reinterpret_cast<const char16_t*>(view.data16()),
-                   view.length())
-            .count;
-      }
-    }
-    return sourceStr->Utf8LengthV2(isolate);
-  }
-  auto source = sourceStr->GetExternalOneByteStringResource();
+  // Even two UTF-8 bytes per Latin-1 code unit fit in a 31-bit Smi.
+  static_assert(String::kMaxLength <= INT32_MAX / 4);
   // For short inputs, the function call overhead to simdutf is maybe
   // not worth it, reserve simdutf for long strings.
-  if (source->length() > 128) {
-    return simdutf::utf8_length_from_latin1(source->data(), source->length());
+  if (source.length > 128) {
+    return static_cast<int32_t>(
+        simdutf::utf8_length_from_latin1(source.data, source.length));
   }
 
-  uint32_t length = source->length();
-  const auto input = reinterpret_cast<const uint8_t*>(source->data());
+  uint32_t length = source.length;
+  const auto input = reinterpret_cast<const uint8_t*>(source.data);
 
   uint32_t answer = length;
   uint32_t i = 0;
@@ -889,7 +877,7 @@ uint32_t FastByteLengthUtf8(
     answer += input[i] >> 7;
   }
 
-  return answer;
+  return static_cast<int32_t>(answer);
 }
 
 static CFunction fast_byte_length_utf8(CFunction::Make(FastByteLengthUtf8));
@@ -2029,10 +2017,11 @@ void Initialize(Local<Object> target,
 
   SetMethod(context, target, "setBufferPrototype", SetBufferPrototype);
 
+  SetMethodNoSideEffect(context, target, "byteLengthUtf8", SlowByteLengthUtf8);
   SetFastMethodNoSideEffect(context,
                             target,
-                            "byteLengthUtf8",
-                            SlowByteLengthUtf8,
+                            "byteLengthUtf8Fast",
+                            ByteLengthUtf8FastFallback,
                             &fast_byte_length_utf8);
   SetFastMethod(context, target, "copy", SlowCopy, &fast_copy);
   SetFastMethodNoSideEffect(context, target, "compare", Compare, &fast_compare);
@@ -2123,6 +2112,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(SetBufferPrototype);
 
   registry->Register(SlowByteLengthUtf8);
+  registry->Register(ByteLengthUtf8FastFallback);
   registry->Register(fast_byte_length_utf8);
   registry->Register(SlowCopy);
   registry->Register(fast_copy);

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

@@ -0,0 +1,35 @@
+// Flags: --allow-natives-syntax --gc-global --gc-interval=500
+'use strict';
+
+require('../common');
+const assert = require('assert');
+
+function byteLength(object, string) {
+  return object.x + Buffer.byteLength(string);
+}
+
+function makeDying() {
+  const object = { x: 1 };
+  object.dying = 1;
+  return object;
+}
+
+const keep = { x: 2, keep: 1 };
+
+// The optimized caller holds the dying objects' map weakly. A full GC while
+// flattening a cons string can invalidate that map and lazily deoptimize it.
+// A value-returning fast API call, including its slow fallback, cannot handle
+// that deoptimization and aborts instead.
+eval('%PrepareFunctionForOptimization(byteLength)');
+for (let i = 0; i < 10; i++) {
+  byteLength(makeDying(), 'ab');
+  byteLength(keep, 'ab');
+}
+eval('%OptimizeFunctionOnNextCall(byteLength)');
+assert.strictEqual(byteLength(keep, 'ab'), 4);
+
+const big = 'a'.repeat(1 << 20);
+for (let i = 0; i < 50; i++) {
+  const cons = big + String(i) + big;
+  assert.strictEqual(byteLength(keep, cons), 2 + cons.length);
+}

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

@@ -0,0 +1,77 @@
+// Flags: --expose-internals --no-warnings --allow-natives-syntax --expose_externalize_string
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const { internalBinding } = require('internal/test/binding');
+const { byteLengthUtf8Fast } = internalBinding('buffer');
+
+const {
+  createExternalizableString,
+  createExternalizableTwoByteString,
+  externalizeString,
+  isOneByteString,
+} = globalThis;
+common.allowGlobals(
+  createExternalizableString,
+  createExternalizableTwoByteString,
+  externalizeString,
+  isOneByteString,
+  globalThis.x,
+);
+
+const flat = ['', 'hello', '\xe9', '\x00\x01\x7f\x80\xff'];
+for (const length of [1, 7, 8, 9, 127, 128, 129, 1024]) {
+  flat.push(Buffer.alloc(length, 0xe9).toString('latin1'));
+}
+const latin1 = flat[flat.length - 1];
+const externalOneByte = createExternalizableString('external é'.repeat(20));
+const externalTwoByte = createExternalizableTwoByteString('external €'.repeat(20));
+externalizeString(externalOneByte);
+externalizeString(externalTwoByte);
+const fallback = [
+  'x'.repeat(1000) + latin1,
+  latin1.slice(1, 1000),
+  '€',
+  '\ud83d',
+  'a€😀'.repeat(200),
+  externalOneByte,
+  externalTwoByte,
+];
+
+function fastLength(string) {
+  return byteLengthUtf8Fast(string);
+}
+
+eval('%PrepareFunctionForOptimization(fastLength)');
+for (const string of [...flat, ...fallback]) {
+  // Before optimization the callback only returns the rejection sentinel.
+  assert.strictEqual(fastLength(string), -1);
+}
+eval('%OptimizeFunctionOnNextCall(fastLength)');
+for (const string of flat) {
+  assert.strictEqual(fastLength(string), Buffer.from(string).length);
+}
+for (const string of fallback) {
+  assert.strictEqual(fastLength(string), -1);
+}
+assert.strictEqual(fastLength('hello'), 5);
+
+if (common.isDebug) {
+  const { getV8FastApiCallCount } = internalBinding('debug');
+  assert.strictEqual(
+    getV8FastApiCallCount('Buffer::FastByteLengthUtf8'), flat.length + 1);
+}
+
+const { byteLength } = Buffer;
+function test() {
+  for (const string of [...flat, ...fallback]) {
+    assert.strictEqual(byteLength(string), Buffer.from(string).length);
+    assert.strictEqual(byteLength(string, 'UTF-8'), Buffer.from(string).length);
+    assert.strictEqual(byteLength(string, 'unknown'), Buffer.from(string).length);
+  }
+}
+eval('%PrepareFunctionForOptimization(byteLength)');
+test();
+eval('%OptimizeFunctionOnNextCall(byteLength)');
+test();

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

@@ -0,0 +1,51 @@
+// Flags: --allow-natives-syntax --expose_externalize_string
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+const {
+  createExternalizableString,
+  createExternalizableTwoByteString,
+  externalizeString,
+  isOneByteString,
+} = globalThis;
+
+common.allowGlobals(
+  createExternalizableString,
+  createExternalizableTwoByteString,
+  externalizeString,
+  isOneByteString,
+  globalThis.x,
+);
+
+const { byteLength } = Buffer;
+const latin1 = Buffer.from('aé'.repeat(100), 'latin1').toString('latin1');
+const external = createExternalizableString('external é'.repeat(20));
+externalizeString(external);
+assert.strictEqual(isOneByteString(external), true);
+
+// Check flat, cons, sliced, two-byte and external strings, including lone
+// surrogates, before and after optimizing the caller.
+const cases = [
+  'hello',
+  'é',
+  latin1,
+  'x'.repeat(1000) + latin1,
+  latin1.slice(1, 150),
+  '€',
+  '\ud83d',
+  'a€😀'.repeat(200),
+  external,
+];
+
+function test() {
+  for (const string of cases) {
+    assert.strictEqual(byteLength(string), Buffer.from(string).length);
+  }
+}
+
+eval('%PrepareFunctionForOptimization(byteLength)');
+test();
+eval('%OptimizeFunctionOnNextCall(byteLength)');
+test();

typings/internalBinding/buffer.d.ts

@@ -5,6 +5,7 @@ export interface BufferBinding {
   setBufferPrototype(proto: object): void;
 
   byteLengthUtf8(str: string): number;
+  byteLengthUtf8Fast(str: string): number;
   copy(source: ArrayBufferView, target: ArrayBufferView, targetStart: number, sourceStart: number, toCopy: number): number;
   compare(a: ArrayBufferView, b: ArrayBufferView): number;
   compareOffset(source: ArrayBufferView, target: ArrayBufferView, targetStart?: number, sourceStart?: number, targetEnd?: number, sourceEnd?: number): number;