Repository: nodejs/node
Pull request: url: improve canParse() performance for non-onebyte strings

V8 Fast API now supports v8::Local<v8::Value> as a parameter type and we can access isolate and context through the fast api. Let's update our implementation to reflect the new limitations.

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1702

```
                                                       confidence improvement accuracy (*)   (**)  (***)
url/whatwg-url-canParse.js n=1000000 type='auth'              ***      5.55 %       ±2.05% ±2.75% ±3.61%
url/whatwg-url-canParse.js n=1000000 type='dot'               ***      3.90 %       ±0.69% ±0.92% ±1.20%
url/whatwg-url-canParse.js n=1000000 type='file'                       1.13 %       ±1.20% ±1.60% ±2.09%
url/whatwg-url-canParse.js n=1000000 type='idn'               ***      2.78 %       ±0.31% ±0.42% ±0.55%
url/whatwg-url-canParse.js n=1000000 type='javascript'        ***      1.85 %       ±0.91% ±1.22% ±1.61%
url/whatwg-url-canParse.js n=1000000 type='long'              ***      9.09 %       ±0.65% ±0.87% ±1.13%
url/whatwg-url-canParse.js n=1000000 type='percent'           ***      1.81 %       ±0.31% ±0.41% ±0.53%
url/whatwg-url-canParse.js n=1000000 type='short'             ***      2.23 %       ±0.90% ±1.20% ±1.58%
url/whatwg-url-canParse.js n=1000000 type='ws'                ***      7.30 %       ±1.65% ±2.20% ±2.87%
```

cc @nodejs/performance

Changed files:
filename: src/node_external_reference.h
patch: @@ -10,6 +10,13 @@
 
 namespace node {
 
+using CFunctionCallbackWithalueAndOptions = bool (*)(
+    v8::Local<v8::Value>, v8::Local<v8::Value>, v8::FastApiCallbackOptions&);
+using CFunctionCallbackWithMultipleValueAndOptions =
+    bool (*)(v8::Local<v8::Value>,
+             v8::Local<v8::Value>,
+             v8::Local<v8::Value>,
+             v8::FastApiCallbackOptions&);
 using CFunctionCallbackWithOneByteString =
     uint32_t (*)(v8::Local<v8::Value>, const v8::FastOneByteString&);
 
@@ -92,6 +99,8 @@ class ExternalReferenceRegistry {
 
 #define ALLOWED_EXTERNAL_REFERENCE_TYPES(V)                                    \
   V(CFunctionCallback)                                                         \
+  V(CFunctionCallbackWithalueAndOptions)                                       \
+  V(CFunctionCallbackWithMultipleValueAndOptions)                              \
   V(CFunctionCallbackWithOneByteString)                                        \
   V(CFunctionCallbackReturnBool)                                               \
   V(CFunctionCallbackReturnDouble)                                             \

filename: src/node_url.cc
patch: @@ -10,6 +10,7 @@
 #include "path.h"
 #include "util-inl.h"
 #include "v8-fast-api-calls.h"
+#include "v8-local-handle.h"
 #include "v8.h"
 
 #include <cstdint>
@@ -21,7 +22,7 @@ namespace url {
 
 using v8::CFunction;
 using v8::Context;
-using v8::FastOneByteString;
+using v8::FastApiCallbackOptions;
 using v8::FunctionCallbackInfo;
 using v8::HandleScope;
 using v8::Isolate;
@@ -282,18 +283,45 @@ void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(can_parse);
 }
 
-bool BindingData::FastCanParse(Local<Value> receiver,
-                               const FastOneByteString& input) {
+bool BindingData::FastCanParse(
+    Local<Value> receiver,
+    Local<Value> input,
+    // NOLINTNEXTLINE(runtime/references) This is V8 api.
+    FastApiCallbackOptions& options) {
   TRACK_V8_FAST_API_CALL("url.canParse");
-  return ada::can_parse(std::string_view(input.data, input.length));
+  auto isolate = options.isolate;
+  HandleScope handleScope(isolate);
+  Local<String> str;
+  if (!input->ToString(isolate->GetCurrentContext()).ToLocal(&str)) {
+    return false;
+  }
+  Utf8Value utf8(isolate, str);
+  return ada::can_parse(utf8.ToStringView());
 }
 
-bool BindingData::FastCanParseWithBase(Local<Value> receiver,
-                                       const FastOneByteString& input,
-                                       const FastOneByteString& base) {
+bool BindingData::FastCanParseWithBase(
+    Local<Value> receiver,
+    Local<Value> input,
+    Local<Value> base,
+    // NOLINTNEXTLINE(runtime/references) This is V8 api.
+    FastApiCallbackOptions& options) {
   TRACK_V8_FAST_API_CALL("url.canParse.withBase");
-  auto base_view = std::string_view(base.data, base.length);
-  return ada::can_parse(std::string_view(input.data, input.length), &base_view);
+  auto isolate = options.isolate;
+  HandleScope handleScope(isolate);
+  auto context = isolate->GetCurrentContext();
+  Local<String> input_str;
+  if (!input->ToString(context).ToLocal(&input_str)) {
+    return false;
+  }
+  Local<String> base_str;
+  if (!base->ToString(context).ToLocal(&base_str)) {
+    return false;
+  }
+  Utf8Value input_utf8(isolate, input_str);
+  Utf8Value base_utf8(isolate, base_str);
+
+  auto base_view = base_utf8.ToStringView();
+  return ada::can_parse(input_utf8.ToStringView(), &base_view);
 }
 
 CFunction BindingData::fast_can_parse_methods_[] = {

filename: src/node_url.h
patch: @@ -51,10 +51,15 @@ class BindingData : public SnapshotableObject {
 
   static void CanParse(const v8::FunctionCallbackInfo<v8::Value>& args);
   static bool FastCanParse(v8::Local<v8::Value> receiver,
-                           const v8::FastOneByteString& input);
-  static bool FastCanParseWithBase(v8::Local<v8::Value> receiver,
-                                   const v8::FastOneByteString& input,
-                                   const v8::FastOneByteString& base);
+                           v8::Local<v8::Value> input,
+                           // NOLINTNEXTLINE(runtime/references) This is V8 api.
+                           v8::FastApiCallbackOptions& options);
+  static bool FastCanParseWithBase(
+      v8::Local<v8::Value> receiver,
+      v8::Local<v8::Value> input,
+      v8::Local<v8::Value> base,
+      // NOLINTNEXTLINE(runtime/references) This is V8 api.
+      v8::FastApiCallbackOptions& options);
 
   static void Format(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetOrigin(const v8::FunctionCallbackInfo<v8::Value>& args);

filename: test/parallel/test-whatwg-url-canparse.js
patch: @@ -20,23 +20,25 @@ assert.throws(() => {
 assert.strictEqual(URL.canParse('https://example.org'), true);
 
 {
-  // V8 Fast API
+  // Only javascript methods can be optimized through %OptimizeFunctionOnNextCall
+  // This is why we surround the C++ method we want to optimize with a JS function.
   function testFastPaths() {
     // `canParse` binding has two overloads.
     assert.strictEqual(URL.canParse('https://www.example.com/path/?query=param#hash'), true);
     assert.strictEqual(URL.canParse('/', 'http://n'), true);
   }
 
+  // Since our JS function contains other javascript functions,
+  // we need to specify which function we want to optimize. This is why
+  // the next line does not optimize "testFastPaths" but "URL.canParse"
   eval('%PrepareFunctionForOptimization(URL.canParse)');
   testFastPaths();
   eval('%OptimizeFunctionOnNextCall(URL.canParse)');
   testFastPaths();
 
   if (common.isDebug) {
     const { getV8FastApiCallCount } = internalBinding('debug');
-    // TODO: the counts should be 1. The function is optimized, but the fast
-    // API is not called.
-    assert.strictEqual(getV8FastApiCallCount('url.canParse'), 0);
-    assert.strictEqual(getV8FastApiCallCount('url.canParse.withBase'), 0);
+    assert.strictEqual(getV8FastApiCallCount('url.canParse'), 1);
+    assert.strictEqual(getV8FastApiCallCount('url.canParse.withBase'), 1);
   }
 }