buffer: avoid fast API calls when computing UTF-8 byte length

src/node_buffer.cc

@@ -826,74 +826,6 @@ 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)
-  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();
-  // 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());
-  }
-
-  uint32_t length = source->length();
-  const auto input = reinterpret_cast<const uint8_t*>(source->data());
-
-  uint32_t answer = length;
-  uint32_t i = 0;
-
-  auto pop = [](uint64_t v) {
-    return static_cast<size_t>(((v >> 7) & UINT64_C(0x0101010101010101)) *
-                                   UINT64_C(0x0101010101010101) >>
-                               56);
-  };
-
-  for (; i + 32 <= length; i += 32) {
-    uint64_t v;
-    memcpy(&v, input + i, 8);
-    answer += pop(v);
-    memcpy(&v, input + i + 8, 8);
-    answer += pop(v);
-    memcpy(&v, input + i + 16, 8);
-    answer += pop(v);
-    memcpy(&v, input + i + 24, 8);
-    answer += pop(v);
-  }
-  for (; i + 8 <= length; i += 8) {
-    uint64_t v;
-    memcpy(&v, input + i, 8);
-    answer += pop(v);
-  }
-  for (; i + 1 <= length; i += 1) {
-    answer += input[i] >> 7;
-  }
-
-  return answer;
-}
-
-static CFunction fast_byte_length_utf8(CFunction::Make(FastByteLengthUtf8));
-
 // Normalize val to be an integer in the range of [1, -1] since
 // implementations of memcmp() can vary by platform.
 static int normalizeCompareVal(int val, size_t a_length, size_t b_length) {
@@ -2029,11 +1961,7 @@ void Initialize(Local<Object> target,
 
   SetMethod(context, target, "setBufferPrototype", SetBufferPrototype);
 
-  SetFastMethodNoSideEffect(context,
-                            target,
-                            "byteLengthUtf8",
-                            SlowByteLengthUtf8,
-                            &fast_byte_length_utf8);
+  SetMethodNoSideEffect(context, target, "byteLengthUtf8", SlowByteLengthUtf8);
   SetFastMethod(context, target, "copy", SlowCopy, &fast_copy);
   SetFastMethodNoSideEffect(context, target, "compare", Compare, &fast_compare);
   SetMethodNoSideEffect(context, target, "compareOffset", CompareOffset);
@@ -2123,7 +2051,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(SetBufferPrototype);
 
   registry->Register(SlowByteLengthUtf8);
-  registry->Register(fast_byte_length_utf8);
   registry->Register(SlowCopy);
   registry->Register(fast_copy);
   registry->Register(Compare);

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-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();