Skip to content

Commit e0cf8ae

Browse files
authored
url: improve canParse() performance for non-onebyte strings
PR-URL: nodejs#58023 Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent 647175e commit e0cf8ae

File tree

4 files changed

+62
-18
lines changed

4 files changed

+62
-18
lines changed

src/node_external_reference.h

+9
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@
1010

1111
namespace node {
1212

13+
using CFunctionCallbackWithalueAndOptions = bool (*)(
14+
v8::Local<v8::Value>, v8::Local<v8::Value>, v8::FastApiCallbackOptions&);
15+
using CFunctionCallbackWithMultipleValueAndOptions =
16+
bool (*)(v8::Local<v8::Value>,
17+
v8::Local<v8::Value>,
18+
v8::Local<v8::Value>,
19+
v8::FastApiCallbackOptions&);
1320
using CFunctionCallbackWithOneByteString =
1421
uint32_t (*)(v8::Local<v8::Value>, const v8::FastOneByteString&);
1522

@@ -92,6 +99,8 @@ class ExternalReferenceRegistry {
9299

93100
#define ALLOWED_EXTERNAL_REFERENCE_TYPES(V) \
94101
V(CFunctionCallback) \
102+
V(CFunctionCallbackWithalueAndOptions) \
103+
V(CFunctionCallbackWithMultipleValueAndOptions) \
95104
V(CFunctionCallbackWithOneByteString) \
96105
V(CFunctionCallbackReturnBool) \
97106
V(CFunctionCallbackReturnDouble) \

src/node_url.cc

+37-9
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "path.h"
1111
#include "util-inl.h"
1212
#include "v8-fast-api-calls.h"
13+
#include "v8-local-handle.h"
1314
#include "v8.h"
1415

1516
#include <cstdint>
@@ -21,7 +22,7 @@ namespace url {
2122

2223
using v8::CFunction;
2324
using v8::Context;
24-
using v8::FastOneByteString;
25+
using v8::FastApiCallbackOptions;
2526
using v8::FunctionCallbackInfo;
2627
using v8::HandleScope;
2728
using v8::Isolate;
@@ -282,18 +283,45 @@ void BindingData::CanParse(const FunctionCallbackInfo<Value>& args) {
282283
args.GetReturnValue().Set(can_parse);
283284
}
284285

285-
bool BindingData::FastCanParse(Local<Value> receiver,
286-
const FastOneByteString& input) {
286+
bool BindingData::FastCanParse(
287+
Local<Value> receiver,
288+
Local<Value> input,
289+
// NOLINTNEXTLINE(runtime/references) This is V8 api.
290+
FastApiCallbackOptions& options) {
287291
TRACK_V8_FAST_API_CALL("url.canParse");
288-
return ada::can_parse(std::string_view(input.data, input.length));
292+
auto isolate = options.isolate;
293+
HandleScope handleScope(isolate);
294+
Local<String> str;
295+
if (!input->ToString(isolate->GetCurrentContext()).ToLocal(&str)) {
296+
return false;
297+
}
298+
Utf8Value utf8(isolate, str);
299+
return ada::can_parse(utf8.ToStringView());
289300
}
290301

291-
bool BindingData::FastCanParseWithBase(Local<Value> receiver,
292-
const FastOneByteString& input,
293-
const FastOneByteString& base) {
302+
bool BindingData::FastCanParseWithBase(
303+
Local<Value> receiver,
304+
Local<Value> input,
305+
Local<Value> base,
306+
// NOLINTNEXTLINE(runtime/references) This is V8 api.
307+
FastApiCallbackOptions& options) {
294308
TRACK_V8_FAST_API_CALL("url.canParse.withBase");
295-
auto base_view = std::string_view(base.data, base.length);
296-
return ada::can_parse(std::string_view(input.data, input.length), &base_view);
309+
auto isolate = options.isolate;
310+
HandleScope handleScope(isolate);
311+
auto context = isolate->GetCurrentContext();
312+
Local<String> input_str;
313+
if (!input->ToString(context).ToLocal(&input_str)) {
314+
return false;
315+
}
316+
Local<String> base_str;
317+
if (!base->ToString(context).ToLocal(&base_str)) {
318+
return false;
319+
}
320+
Utf8Value input_utf8(isolate, input_str);
321+
Utf8Value base_utf8(isolate, base_str);
322+
323+
auto base_view = base_utf8.ToStringView();
324+
return ada::can_parse(input_utf8.ToStringView(), &base_view);
297325
}
298326

299327
CFunction BindingData::fast_can_parse_methods_[] = {

src/node_url.h

+9-4
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,15 @@ class BindingData : public SnapshotableObject {
5151

5252
static void CanParse(const v8::FunctionCallbackInfo<v8::Value>& args);
5353
static bool FastCanParse(v8::Local<v8::Value> receiver,
54-
const v8::FastOneByteString& input);
55-
static bool FastCanParseWithBase(v8::Local<v8::Value> receiver,
56-
const v8::FastOneByteString& input,
57-
const v8::FastOneByteString& base);
54+
v8::Local<v8::Value> input,
55+
// NOLINTNEXTLINE(runtime/references) This is V8 api.
56+
v8::FastApiCallbackOptions& options);
57+
static bool FastCanParseWithBase(
58+
v8::Local<v8::Value> receiver,
59+
v8::Local<v8::Value> input,
60+
v8::Local<v8::Value> base,
61+
// NOLINTNEXTLINE(runtime/references) This is V8 api.
62+
v8::FastApiCallbackOptions& options);
5863

5964
static void Format(const v8::FunctionCallbackInfo<v8::Value>& args);
6065
static void GetOrigin(const v8::FunctionCallbackInfo<v8::Value>& args);

test/parallel/test-whatwg-url-canparse.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,25 @@ assert.throws(() => {
2020
assert.strictEqual(URL.canParse('https://example.org'), true);
2121

2222
{
23-
// V8 Fast API
23+
// Only javascript methods can be optimized through %OptimizeFunctionOnNextCall
24+
// This is why we surround the C++ method we want to optimize with a JS function.
2425
function testFastPaths() {
2526
// `canParse` binding has two overloads.
2627
assert.strictEqual(URL.canParse('https://www.example.com/path/?query=param#hash'), true);
2728
assert.strictEqual(URL.canParse('/', 'http://n'), true);
2829
}
2930

31+
// Since our JS function contains other javascript functions,
32+
// we need to specify which function we want to optimize. This is why
33+
// the next line does not optimize "testFastPaths" but "URL.canParse"
3034
eval('%PrepareFunctionForOptimization(URL.canParse)');
3135
testFastPaths();
3236
eval('%OptimizeFunctionOnNextCall(URL.canParse)');
3337
testFastPaths();
3438

3539
if (common.isDebug) {
3640
const { getV8FastApiCallCount } = internalBinding('debug');
37-
// TODO: the counts should be 1. The function is optimized, but the fast
38-
// API is not called.
39-
assert.strictEqual(getV8FastApiCallCount('url.canParse'), 0);
40-
assert.strictEqual(getV8FastApiCallCount('url.canParse.withBase'), 0);
41+
assert.strictEqual(getV8FastApiCallCount('url.canParse'), 1);
42+
assert.strictEqual(getV8FastApiCallCount('url.canParse.withBase'), 1);
4143
}
4244
}

0 commit comments

Comments
 (0)