From 608965d1ed67e07a9c4e4396c79080ae941f36e8 Mon Sep 17 00:00:00 2001 From: Khafra Date: Thu, 13 Apr 2023 15:19:57 -0400 Subject: [PATCH 1/6] src: add v8 fast api for url canParse --- src/node_external_reference.h | 4 ++++ src/node_url.cc | 32 +++++++++++++++++++++++++++++--- src/node_url.h | 7 +++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/node_external_reference.h b/src/node_external_reference.h index b2a90ba5194316..58453066e30292 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -19,6 +19,9 @@ using CFunctionCallbackWithInt64 = void (*)(v8::Local receiver, int64_t); using CFunctionCallbackWithBool = void (*)(v8::Local receiver, bool); +using CFunctionCallbackWithStrings = + bool (*)(const v8::FastOneByteString& input, + const v8::FastOneByteString& base); // This class manages the external references from the V8 heap // to the C++ addresses in Node.js. @@ -32,6 +35,7 @@ class ExternalReferenceRegistry { V(CFunctionCallbackReturnDouble) \ V(CFunctionCallbackWithInt64) \ V(CFunctionCallbackWithBool) \ + V(CFunctionCallbackWithStrings) \ V(const v8::CFunctionInfo*) \ V(v8::FunctionCallback) \ V(v8::AccessorGetterCallback) \ diff --git a/src/node_url.cc b/src/node_url.cc index 463de5f268ee95..2dd5620568ba59 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -6,6 +6,7 @@ #include "node_i18n.h" #include "util-inl.h" #include "v8.h" +#include "v8-fast-api-calls.h" #include #include @@ -14,7 +15,9 @@ namespace node { namespace url { +using v8::CFunction; using v8::Context; +using v8::FastOneByteString; using v8::FunctionCallbackInfo; using v8::HandleScope; using v8::Isolate; @@ -113,7 +116,6 @@ void BindingData::DomainToUnicode(const FunctionCallbackInfo& args) { .ToLocalChecked()); } -// TODO(@anonrig): Add V8 Fast API for CanParse method void BindingData::CanParse(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 2); CHECK(args[0]->IsString()); // input @@ -140,6 +142,28 @@ void BindingData::CanParse(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(out.has_value()); } +bool BindingData::FastCanParse(const FastOneByteString& input, + const FastOneByteString& base) { + std::string_view input_view(input.data, input.length); + std::string_view base_view(base.data, base.length); + + ada::result bases = + ada::parse(base_view); + + if (!bases) { + return false; + } + + ada::url_aggregator* base_pointer = &bases.value(); + + auto out = + ada::parse(input_view, base_pointer); + + return out.has_value(); +} + +CFunction BindingData::fast_canParse_(CFunction::Make(FastCanParse)); + void BindingData::Format(const FunctionCallbackInfo& args) { CHECK_GT(args.Length(), 4); CHECK(args[0]->IsString()); // url href @@ -320,20 +344,22 @@ void BindingData::Initialize(Local target, SetMethodNoSideEffect(context, target, "domainToASCII", DomainToASCII); SetMethodNoSideEffect(context, target, "domainToUnicode", DomainToUnicode); - SetMethodNoSideEffect(context, target, "canParse", CanParse); SetMethodNoSideEffect(context, target, "format", Format); SetMethod(context, target, "parse", Parse); SetMethod(context, target, "update", Update); + SetFastMethod(context, target, "canParse", CanParse, &fast_canParse_); } void BindingData::RegisterExternalReferences( ExternalReferenceRegistry* registry) { registry->Register(DomainToASCII); registry->Register(DomainToUnicode); - registry->Register(CanParse); registry->Register(Format); registry->Register(Parse); registry->Register(Update); + registry->Register(CanParse); + registry->Register(FastCanParse); + registry->Register(fast_canParse_.GetTypeInfo()); } std::string FromFilePath(const std::string_view file_path) { diff --git a/src/node_url.h b/src/node_url.h index ed68fe51fb4d10..640a8e7d756a2a 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -9,6 +9,8 @@ #include "node.h" #include "node_snapshotable.h" #include "util.h" +#include "v8.h" +#include "v8-fast-api-calls.h" #include @@ -47,6 +49,9 @@ class BindingData : public SnapshotableObject { static void DomainToUnicode(const v8::FunctionCallbackInfo& args); static void CanParse(const v8::FunctionCallbackInfo& args); + static bool FastCanParse(const v8::FastOneByteString& input, + const v8::FastOneByteString& base); + static void Format(const v8::FunctionCallbackInfo& args); static void Parse(const v8::FunctionCallbackInfo& args); static void Update(const v8::FunctionCallbackInfo& args); @@ -63,6 +68,8 @@ class BindingData : public SnapshotableObject { void UpdateComponents(const ada::url_components& components, const ada::scheme::type type); + + static v8::CFunction fast_canParse_; }; std::string FromFilePath(const std::string_view file_path); From f3e308bc5ef1ca9dcb6dade92a7d4573ec6713ec Mon Sep 17 00:00:00 2001 From: Khafra Date: Thu, 13 Apr 2023 16:24:35 -0400 Subject: [PATCH 2/6] src: fix fast api param order --- src/node_external_reference.h | 3 ++- src/node_url.cc | 3 ++- src/node_url.h | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 58453066e30292..80a45824537537 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -20,7 +20,8 @@ using CFunctionCallbackWithInt64 = void (*)(v8::Local receiver, using CFunctionCallbackWithBool = void (*)(v8::Local receiver, bool); using CFunctionCallbackWithStrings = - bool (*)(const v8::FastOneByteString& input, + bool (*)(v8::Local, + const v8::FastOneByteString& input, const v8::FastOneByteString& base); // This class manages the external references from the V8 heap diff --git a/src/node_url.cc b/src/node_url.cc index 2dd5620568ba59..bef415125fedc9 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -142,7 +142,8 @@ void BindingData::CanParse(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(out.has_value()); } -bool BindingData::FastCanParse(const FastOneByteString& input, +bool BindingData::FastCanParse(Local receiver, + const FastOneByteString& input, const FastOneByteString& base) { std::string_view input_view(input.data, input.length); std::string_view base_view(base.data, base.length); diff --git a/src/node_url.h b/src/node_url.h index 640a8e7d756a2a..1192e3c438b080 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -49,7 +49,8 @@ class BindingData : public SnapshotableObject { static void DomainToUnicode(const v8::FunctionCallbackInfo& args); static void CanParse(const v8::FunctionCallbackInfo& args); - static bool FastCanParse(const v8::FastOneByteString& input, + static bool FastCanParse(v8::Local receiver, + const v8::FastOneByteString& input, const v8::FastOneByteString& base); static void Format(const v8::FunctionCallbackInfo& args); From 8e769eab20a33b5b511ae3083077626b2211de1c Mon Sep 17 00:00:00 2001 From: Khafra Date: Thu, 13 Apr 2023 16:35:38 -0400 Subject: [PATCH 3/6] src: add fast path for input w/o base --- src/node_external_reference.h | 3 +-- src/node_url.cc | 21 +++++---------------- src/node_url.h | 3 +-- 3 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 80a45824537537..e756ebc316e06b 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -21,8 +21,7 @@ using CFunctionCallbackWithBool = void (*)(v8::Local receiver, bool); using CFunctionCallbackWithStrings = bool (*)(v8::Local, - const v8::FastOneByteString& input, - const v8::FastOneByteString& base); + const v8::FastOneByteString& input); // This class manages the external references from the V8 heap // to the C++ addresses in Node.js. diff --git a/src/node_url.cc b/src/node_url.cc index bef415125fedc9..cbbb1252ab7e89 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -143,24 +143,13 @@ void BindingData::CanParse(const FunctionCallbackInfo& args) { } bool BindingData::FastCanParse(Local receiver, - const FastOneByteString& input, - const FastOneByteString& base) { + const FastOneByteString& input) { std::string_view input_view(input.data, input.length); - std::string_view base_view(base.data, base.length); - ada::result bases = - ada::parse(base_view); + ada::result output = + ada::parse(input_view); - if (!bases) { - return false; - } - - ada::url_aggregator* base_pointer = &bases.value(); - - auto out = - ada::parse(input_view, base_pointer); - - return out.has_value(); + return output.has_value(); } CFunction BindingData::fast_canParse_(CFunction::Make(FastCanParse)); @@ -348,7 +337,7 @@ void BindingData::Initialize(Local target, SetMethodNoSideEffect(context, target, "format", Format); SetMethod(context, target, "parse", Parse); SetMethod(context, target, "update", Update); - SetFastMethod(context, target, "canParse", CanParse, &fast_canParse_); + SetFastMethodNoSideEffect(context, target, "canParse", CanParse, &fast_canParse_); } void BindingData::RegisterExternalReferences( diff --git a/src/node_url.h b/src/node_url.h index 1192e3c438b080..5efbf7ab66d41e 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -50,8 +50,7 @@ class BindingData : public SnapshotableObject { static void CanParse(const v8::FunctionCallbackInfo& args); static bool FastCanParse(v8::Local receiver, - const v8::FastOneByteString& input, - const v8::FastOneByteString& base); + const v8::FastOneByteString& input); static void Format(const v8::FunctionCallbackInfo& args); static void Parse(const v8::FunctionCallbackInfo& args); From 7bef65fcdeee6eefe4daa95f339df9316c14b084 Mon Sep 17 00:00:00 2001 From: Khafra Date: Thu, 13 Apr 2023 16:58:13 -0400 Subject: [PATCH 4/6] src: lint --- src/node_url.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_url.cc b/src/node_url.cc index cbbb1252ab7e89..64c86e4a186fc1 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -337,7 +337,8 @@ void BindingData::Initialize(Local target, SetMethodNoSideEffect(context, target, "format", Format); SetMethod(context, target, "parse", Parse); SetMethod(context, target, "update", Update); - SetFastMethodNoSideEffect(context, target, "canParse", CanParse, &fast_canParse_); + SetFastMethodNoSideEffect( + context, target, "canParse", CanParse, &fast_canParse_); } void BindingData::RegisterExternalReferences( From 877f1fba29cf1cb24c29880e42b4f9b7546b8273 Mon Sep 17 00:00:00 2001 From: Khafra Date: Thu, 13 Apr 2023 17:06:42 -0400 Subject: [PATCH 5/6] src: format cpp --- src/node_external_reference.h | 3 +-- src/node_url.cc | 4 ++-- src/node_url.h | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/node_external_reference.h b/src/node_external_reference.h index e756ebc316e06b..85acbc114143da 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -20,8 +20,7 @@ using CFunctionCallbackWithInt64 = void (*)(v8::Local receiver, using CFunctionCallbackWithBool = void (*)(v8::Local receiver, bool); using CFunctionCallbackWithStrings = - bool (*)(v8::Local, - const v8::FastOneByteString& input); + bool (*)(v8::Local, const v8::FastOneByteString& input); // This class manages the external references from the V8 heap // to the C++ addresses in Node.js. diff --git a/src/node_url.cc b/src/node_url.cc index 64c86e4a186fc1..c3a62c555b8765 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -5,8 +5,8 @@ #include "node_external_reference.h" #include "node_i18n.h" #include "util-inl.h" -#include "v8.h" #include "v8-fast-api-calls.h" +#include "v8.h" #include #include @@ -338,7 +338,7 @@ void BindingData::Initialize(Local target, SetMethod(context, target, "parse", Parse); SetMethod(context, target, "update", Update); SetFastMethodNoSideEffect( - context, target, "canParse", CanParse, &fast_canParse_); + context, target, "canParse", CanParse, &fast_canParse_); } void BindingData::RegisterExternalReferences( diff --git a/src/node_url.h b/src/node_url.h index 5efbf7ab66d41e..5caf8ffb888a17 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -9,8 +9,8 @@ #include "node.h" #include "node_snapshotable.h" #include "util.h" -#include "v8.h" #include "v8-fast-api-calls.h" +#include "v8.h" #include From 34ac4e8f7bc4effb4b919379d016d9610aae5b66 Mon Sep 17 00:00:00 2001 From: Khafra Date: Thu, 13 Apr 2023 17:30:00 -0400 Subject: [PATCH 6/6] src,test: address pr comments --- src/node_url.cc | 9 ++++----- src/node_url.h | 2 +- test/parallel/test-url-canParse-whatwg.js | 7 +++++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/node_url.cc b/src/node_url.cc index c3a62c555b8765..400e4263931570 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -146,13 +146,12 @@ bool BindingData::FastCanParse(Local receiver, const FastOneByteString& input) { std::string_view input_view(input.data, input.length); - ada::result output = - ada::parse(input_view); + auto output = ada::parse(input_view); return output.has_value(); } -CFunction BindingData::fast_canParse_(CFunction::Make(FastCanParse)); +CFunction BindingData::fast_can_parse_(CFunction::Make(FastCanParse)); void BindingData::Format(const FunctionCallbackInfo& args) { CHECK_GT(args.Length(), 4); @@ -338,7 +337,7 @@ void BindingData::Initialize(Local target, SetMethod(context, target, "parse", Parse); SetMethod(context, target, "update", Update); SetFastMethodNoSideEffect( - context, target, "canParse", CanParse, &fast_canParse_); + context, target, "canParse", CanParse, &fast_can_parse_); } void BindingData::RegisterExternalReferences( @@ -350,7 +349,7 @@ void BindingData::RegisterExternalReferences( registry->Register(Update); registry->Register(CanParse); registry->Register(FastCanParse); - registry->Register(fast_canParse_.GetTypeInfo()); + registry->Register(fast_can_parse_.GetTypeInfo()); } std::string FromFilePath(const std::string_view file_path) { diff --git a/src/node_url.h b/src/node_url.h index 5caf8ffb888a17..735a191e5eac16 100644 --- a/src/node_url.h +++ b/src/node_url.h @@ -69,7 +69,7 @@ class BindingData : public SnapshotableObject { void UpdateComponents(const ada::url_components& components, const ada::scheme::type type); - static v8::CFunction fast_canParse_; + static v8::CFunction fast_can_parse_; }; std::string FromFilePath(const std::string_view file_path); diff --git a/test/parallel/test-url-canParse-whatwg.js b/test/parallel/test-url-canParse-whatwg.js index 7f7d5d40aa7a28..d5ffee7053a716 100644 --- a/test/parallel/test-url-canParse-whatwg.js +++ b/test/parallel/test-url-canParse-whatwg.js @@ -10,3 +10,10 @@ assert.throws(() => { code: 'ERR_MISSING_ARGS', name: 'TypeError', }); + +{ + // This test is to ensure that the v8 fast api works. + for (let i = 0; i < 1e5; i++) { + assert(URL.canParse('https://www.example.com/path/?query=param#hash')); + } +}