buffer: address PR feedback

- Guard ensureUint8ArrayToHex against --no-js-base-64 flag by
  falling back to C++ hexSlice when toHex is unavailable
- Remove THROW_AND_RETURN_UNLESS_BUFFER and return value from
  slow Swap16/32/64 to match fast path conventions (JS validates)
- Add TRACK_V8_FAST_API_CALL to FastSwap16/32/64
- Add test/parallel/test-buffer-swap-fast.js for fast API verification

Author: thisalihassan

lib/buffer.js

@@ -56,13 +56,13 @@ const {
   uncurryThis,
 } = primordials;
 
-// V8 shipping feature (toHex) is installed by
-// InitializeExperimentalGlobal(), which is skipped during snapshot creation
-// TODO: Remove this once V8 shipping feature (toHex) is available.
+// Lazily initialized: toHex is installed by InitializeExperimentalGlobal()
+// (skipped during snapshot creation) and may be disabled with --no-js-base-64.
 let Uint8ArrayPrototypeToHex;
 function ensureUint8ArrayToHex() {
   if (Uint8ArrayPrototypeToHex === undefined) {
-    Uint8ArrayPrototypeToHex = uncurryThis(Uint8ArrayPrototype.toHex);
+    Uint8ArrayPrototypeToHex = Uint8ArrayPrototype.toHex ?
+      uncurryThis(Uint8ArrayPrototype.toHex) : null;
   }
 }
 
@@ -715,6 +715,10 @@ function base64ByteLength(str, bytes) {
 
 function hexSliceToHex(buf, start, end) {
   ensureUint8ArrayToHex();
+  // Fall back to C++ when toHex is unavailable or the result would exceed
+  // kStringMaxLength (so the correct ERR_STRING_TOO_LONG error is thrown).
+  if (Uint8ArrayPrototypeToHex === null || (end - start) * 2 > kStringMaxLength)
+    return hexSlice(buf, start, end);
   return Uint8ArrayPrototypeToHex(
     new Uint8Array(TypedArrayPrototypeGetBuffer(buf),
                    TypedArrayPrototypeGetByteOffset(buf) + start,

src/node_buffer.cc

@@ -1196,17 +1196,15 @@ int32_t FastIndexOfNumber(Local<Value>,
 static CFunction fast_index_of_number(CFunction::Make(FastIndexOfNumber));
 
 void Swap16(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   SPREAD_BUFFER_ARG(args[0], ts_obj);
   CHECK(nbytes::SwapBytes16(ts_obj_data, ts_obj_length));
-  args.GetReturnValue().Set(args[0]);
 }
 
 void FastSwap16(Local<Value> receiver,
                 Local<Value> buffer_obj,
                 // NOLINTNEXTLINE(runtime/references)
                 FastApiCallbackOptions& options) {
+  TRACK_V8_FAST_API_CALL("buffer.swap16");
   HandleScope scope(options.isolate);
   ArrayBufferViewContents<char> buffer(buffer_obj);
   CHECK(nbytes::SwapBytes16(const_cast<char*>(buffer.data()), buffer.length()));
@@ -1215,17 +1213,15 @@ void FastSwap16(Local<Value> receiver,
 static CFunction fast_swap16(CFunction::Make(FastSwap16));
 
 void Swap32(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   SPREAD_BUFFER_ARG(args[0], ts_obj);
   CHECK(nbytes::SwapBytes32(ts_obj_data, ts_obj_length));
-  args.GetReturnValue().Set(args[0]);
 }
 
 void FastSwap32(Local<Value> receiver,
                 Local<Value> buffer_obj,
                 // NOLINTNEXTLINE(runtime/references)
                 FastApiCallbackOptions& options) {
+  TRACK_V8_FAST_API_CALL("buffer.swap32");
   HandleScope scope(options.isolate);
   ArrayBufferViewContents<char> buffer(buffer_obj);
   CHECK(nbytes::SwapBytes32(const_cast<char*>(buffer.data()), buffer.length()));
@@ -1234,17 +1230,15 @@ void FastSwap32(Local<Value> receiver,
 static CFunction fast_swap32(CFunction::Make(FastSwap32));
 
 void Swap64(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
   SPREAD_BUFFER_ARG(args[0], ts_obj);
   CHECK(nbytes::SwapBytes64(ts_obj_data, ts_obj_length));
-  args.GetReturnValue().Set(args[0]);
 }
 
 void FastSwap64(Local<Value> receiver,
                 Local<Value> buffer_obj,
                 // NOLINTNEXTLINE(runtime/references)
                 FastApiCallbackOptions& options) {
+  TRACK_V8_FAST_API_CALL("buffer.swap64");
   HandleScope scope(options.isolate);
   ArrayBufferViewContents<char> buffer(buffer_obj);
   CHECK(nbytes::SwapBytes64(const_cast<char*>(buffer.data()), buffer.length()));

test/parallel/test-buffer-swap-fast.js

@@ -0,0 +1,43 @@
+// Flags: --expose-internals --no-warnings --allow-natives-syntax
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+
+function testFastSwap16() {
+  const buf = Buffer.alloc(256);
+  buf.swap16();
+}
+
+function testFastSwap32() {
+  const buf = Buffer.alloc(256);
+  buf.swap32();
+}
+
+function testFastSwap64() {
+  const buf = Buffer.alloc(256);
+  buf.swap64();
+}
+
+eval('%PrepareFunctionForOptimization(Buffer.prototype.swap16)');
+testFastSwap16();
+eval('%OptimizeFunctionOnNextCall(Buffer.prototype.swap16)');
+testFastSwap16();
+
+eval('%PrepareFunctionForOptimization(Buffer.prototype.swap32)');
+testFastSwap32();
+eval('%OptimizeFunctionOnNextCall(Buffer.prototype.swap32)');
+testFastSwap32();
+
+eval('%PrepareFunctionForOptimization(Buffer.prototype.swap64)');
+testFastSwap64();
+eval('%OptimizeFunctionOnNextCall(Buffer.prototype.swap64)');
+testFastSwap64();
+
+if (common.isDebug) {
+  const { internalBinding } = require('internal/test/binding');
+  const { getV8FastApiCallCount } = internalBinding('debug');
+  assert.strictEqual(getV8FastApiCallCount('buffer.swap16'), 1);
+  assert.strictEqual(getV8FastApiCallCount('buffer.swap32'), 1);
+  assert.strictEqual(getV8FastApiCallCount('buffer.swap64'), 1);
+}