From 77fd7b8c6c42f95dcdd80b07a8f50f4e1e3704f1 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Thu, 29 May 2025 20:16:24 +0100 Subject: [PATCH 1/5] src: add V8 Fast API for URL.revokeObjectURL co-authored-by: Nicholas Paun --- src/node_blob.cc | 47 ++++++++++++++++++- src/node_blob.h | 13 +++-- src/node_external_reference.h | 8 +++- .../parallel/test-url-revokeobjecturl-fast.js | 26 ++++++++++ 4 files changed, 86 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-url-revokeobjecturl-fast.js diff --git a/src/node_blob.cc b/src/node_blob.cc index e8c0795fcaac60..1f9172ad2f41d7 100644 --- a/src/node_blob.cc +++ b/src/node_blob.cc @@ -5,12 +5,15 @@ #include "env-inl.h" #include "memory_tracker-inl.h" #include "node_bob-inl.h" +#include "node_debug.h" #include "node_errors.h" #include "node_external_reference.h" #include "node_file.h" #include "path.h" #include "permission/permission.h" #include "util.h" +#include "v8-fast-api-calls.h" +#include "v8-value.h" #include "v8.h" #include @@ -22,7 +25,9 @@ using v8::ArrayBuffer; using v8::ArrayBufferView; using v8::BackingStore; using v8::BackingStoreInitializationMode; +using v8::CFunction; using v8::Context; +using v8::FastApiCallbackOptions; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -130,7 +135,11 @@ void Blob::CreatePerIsolateProperties(IsolateData* isolate_data, SetMethod(isolate, target, "createBlob", New); SetMethod(isolate, target, "storeDataObject", StoreDataObject); SetMethod(isolate, target, "getDataObject", GetDataObject); - SetMethod(isolate, target, "revokeObjectURL", RevokeObjectURL); + SetFastMethod(isolate, + target, + "revokeObjectURL", + RevokeObjectURL, + &fast_revoke_object_url_method); SetMethod(isolate, target, "concat", Concat); SetMethod(isolate, target, "createBlobFromFilePath", BlobFromFilePath); } @@ -450,7 +459,6 @@ void Blob::StoreDataObject(const FunctionCallbackInfo& args) { std::string(*type, type.length()))); } -// TODO(@anonrig): Add V8 Fast API to the following function void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 1); CHECK(args[0]->IsString()); @@ -477,6 +485,39 @@ void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { } } +void Blob::FastRevokeObjectURL(Local receiver, + Local raw_input, + FastApiCallbackOptions& options) { + TRACK_V8_FAST_API_CALL("blob.revokeObjectURL"); + CHECK(raw_input->IsString()); + + auto isolate = options.isolate; + HandleScope handleScope(isolate); + Realm* realm = Realm::GetCurrent(isolate->GetCurrentContext()); + BlobBindingData* binding_data = realm->GetBindingData(); + + Utf8Value input(isolate, raw_input.As()); + auto out = ada::parse(input.ToStringView()); + + if (!out) { + return; + } + + auto pathname = out->get_pathname(); + auto start_index = pathname.find(':'); + + if (start_index != std::string_view::npos && start_index != pathname.size()) { + auto end_index = pathname.find(':', start_index + 1); + if (end_index == std::string_view::npos) { + auto id = std::string(pathname.substr(start_index + 1)); + binding_data->revoke_data_object(id); + } + } +} + +CFunction Blob::fast_revoke_object_url_method = + CFunction::Make(Blob::FastRevokeObjectURL); + void Blob::GetDataObject(const FunctionCallbackInfo& args) { CHECK(args[0]->IsString()); Realm* realm = Realm::GetCurrent(args); @@ -584,9 +625,11 @@ void Blob::RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Blob::StoreDataObject); registry->Register(Blob::GetDataObject); registry->Register(Blob::RevokeObjectURL); + registry->Register(Blob::FastRevokeObjectURL); registry->Register(Blob::Reader::Pull); registry->Register(Concat); registry->Register(BlobFromFilePath); + registry->Register(fast_revoke_object_url_method.GetTypeInfo()); } } // namespace node diff --git a/src/node_blob.h b/src/node_blob.h index c601015d9af47b..e7812fdc89a11d 100644 --- a/src/node_blob.h +++ b/src/node_blob.h @@ -5,6 +5,9 @@ #include "v8-template.h" #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include +#include +#include #include "async_wrap.h" #include "base_object.h" #include "dataqueue/queue.h" @@ -13,12 +16,9 @@ #include "node_internals.h" #include "node_snapshotable.h" #include "node_worker.h" +#include "v8-fast-api-calls.h" #include "v8.h" -#include -#include -#include - namespace node { class Blob : public BaseObject { @@ -39,6 +39,11 @@ class Blob : public BaseObject { static void StoreDataObject(const v8::FunctionCallbackInfo& args); static void GetDataObject(const v8::FunctionCallbackInfo& args); static void RevokeObjectURL(const v8::FunctionCallbackInfo& args); + static void FastRevokeObjectURL(v8::Local receiver, + v8::Local raw_input, + v8::FastApiCallbackOptions& options); + + static v8::CFunction fast_revoke_object_url_method; static v8::Local GetConstructorTemplate( Environment* env); diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 5981e9db9c3bc4..d2867a6cad1bb9 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -10,7 +10,9 @@ namespace node { -using CFunctionCallbackWithalueAndOptions = bool (*)( +using CFunctionCallbackWithValueAndOptions = void (*)( + v8::Local, v8::Local, v8::FastApiCallbackOptions&); +using CFunctionCallbackWithValueAndOptionsReturnBool = bool (*)( v8::Local, v8::Local, v8::FastApiCallbackOptions&); using CFunctionCallbackWithMultipleValueAndOptions = bool (*)(v8::Local, @@ -46,6 +48,7 @@ using CFunctionCallbackWithInt64 = void (*)(v8::Local unused, using CFunctionCallbackWithBool = void (*)(v8::Local unused, v8::Local receiver, bool); + using CFunctionCallbackWithString = bool (*)(v8::Local, const v8::FastOneByteString& input); using CFunctionCallbackWithStrings = @@ -103,7 +106,8 @@ class ExternalReferenceRegistry { #define ALLOWED_EXTERNAL_REFERENCE_TYPES(V) \ V(CFunctionA) \ V(CFunctionCallback) \ - V(CFunctionCallbackWithalueAndOptions) \ + V(CFunctionCallbackWithValueAndOptions) \ + V(CFunctionCallbackWithValueAndOptionsReturnBool) \ V(CFunctionCallbackWithMultipleValueAndOptions) \ V(CFunctionCallbackWithOneByteString) \ V(CFunctionCallbackReturnBool) \ diff --git a/test/parallel/test-url-revokeobjecturl-fast.js b/test/parallel/test-url-revokeobjecturl-fast.js new file mode 100644 index 00000000000000..9099c658b8aebe --- /dev/null +++ b/test/parallel/test-url-revokeobjecturl-fast.js @@ -0,0 +1,26 @@ +// Flags: --expose-internals --allow-natives-syntax +'use strict'; +const common = require('../common'); +const assert = require('node:assert'); + +const { internalBinding } = require('internal/test/binding'); + +const blob = new Blob([JSON.stringify({ hello: 'world' }, null, 2)], { + type: 'application/json', +}); + +function testFastPath() { + const objURL = URL.createObjectURL(blob); + URL.revokeObjectURL(objURL); +} + +eval('%PrepareFunctionForOptimization(URL.revokeObjectURL)'); +testFastPath(); + +eval('%OptimizeFunctionOnNextCall(URL.revokeObjectURL)'); +testFastPath(); + +if (common.isDebug) { + const { getV8FastApiCallCount } = internalBinding('debug'); + assert.strictEqual(getV8FastApiCallCount('blob.revokeObjectURL'), 1); +} From 60a7595dea06626a7f7f6c50c399a83fe010d225 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sun, 1 Jun 2025 12:34:51 +0100 Subject: [PATCH 2/5] fixup! src: add V8 Fast API for URL.revokeObjectURL silence cpp lint error --- src/node_blob.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node_blob.h b/src/node_blob.h index e7812fdc89a11d..103f59c60f1aaa 100644 --- a/src/node_blob.h +++ b/src/node_blob.h @@ -41,6 +41,7 @@ class Blob : public BaseObject { static void RevokeObjectURL(const v8::FunctionCallbackInfo& args); static void FastRevokeObjectURL(v8::Local receiver, v8::Local raw_input, + // NOLINTNEXTLINE(runtime/references) This is V8 api. v8::FastApiCallbackOptions& options); static v8::CFunction fast_revoke_object_url_method; From e947f449a397676402822330aa8dac969d03ba72 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sun, 1 Jun 2025 12:37:11 +0100 Subject: [PATCH 3/5] fixup! src: add V8 Fast API for URL.revokeObjectURL add RevokeObjectURLImpl to share logic --- src/node_blob.cc | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/src/node_blob.cc b/src/node_blob.cc index 1f9172ad2f41d7..9ac6f42962b2f3 100644 --- a/src/node_blob.cc +++ b/src/node_blob.cc @@ -459,14 +459,11 @@ void Blob::StoreDataObject(const FunctionCallbackInfo& args) { std::string(*type, type.length()))); } -void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { - CHECK_GE(args.Length(), 1); - CHECK(args[0]->IsString()); - Realm* realm = Realm::GetCurrent(args); +void RevokeObjectURLImpl(Realm* realm, Local input_str) { BlobBindingData* binding_data = realm->GetBindingData(); Isolate* isolate = realm->isolate(); - Utf8Value input(isolate, args[0].As()); + Utf8Value input(isolate, input_str); auto out = ada::parse(input.ToStringView()); if (!out) { @@ -485,34 +482,22 @@ void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { } } +void Blob::RevokeObjectURL(const FunctionCallbackInfo& args) { + CHECK_GE(args.Length(), 1); + CHECK(args[0]->IsString()); + Realm* realm = Realm::GetCurrent(args); + RevokeObjectURLImpl(realm, args[0].As()); +} + void Blob::FastRevokeObjectURL(Local receiver, Local raw_input, FastApiCallbackOptions& options) { TRACK_V8_FAST_API_CALL("blob.revokeObjectURL"); CHECK(raw_input->IsString()); - auto isolate = options.isolate; HandleScope handleScope(isolate); Realm* realm = Realm::GetCurrent(isolate->GetCurrentContext()); - BlobBindingData* binding_data = realm->GetBindingData(); - - Utf8Value input(isolate, raw_input.As()); - auto out = ada::parse(input.ToStringView()); - - if (!out) { - return; - } - - auto pathname = out->get_pathname(); - auto start_index = pathname.find(':'); - - if (start_index != std::string_view::npos && start_index != pathname.size()) { - auto end_index = pathname.find(':', start_index + 1); - if (end_index == std::string_view::npos) { - auto id = std::string(pathname.substr(start_index + 1)); - binding_data->revoke_data_object(id); - } - } + RevokeObjectURLImpl(realm, raw_input.As()); } CFunction Blob::fast_revoke_object_url_method = From f26dd82053628c87326319149589bc61fd3b9c01 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sun, 1 Jun 2025 12:40:09 +0100 Subject: [PATCH 4/5] fixup! src: add V8 Fast API for URL.revokeObjectURL fix cpp formatting --- src/node_blob.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/node_blob.h b/src/node_blob.h index 103f59c60f1aaa..f4baa8d0297d54 100644 --- a/src/node_blob.h +++ b/src/node_blob.h @@ -39,10 +39,11 @@ class Blob : public BaseObject { static void StoreDataObject(const v8::FunctionCallbackInfo& args); static void GetDataObject(const v8::FunctionCallbackInfo& args); static void RevokeObjectURL(const v8::FunctionCallbackInfo& args); - static void FastRevokeObjectURL(v8::Local receiver, - v8::Local raw_input, - // NOLINTNEXTLINE(runtime/references) This is V8 api. - v8::FastApiCallbackOptions& options); + static void FastRevokeObjectURL( + v8::Local receiver, + v8::Local raw_input, + // NOLINTNEXTLINE(runtime/references) This is V8 api. + v8::FastApiCallbackOptions& options); static v8::CFunction fast_revoke_object_url_method; From 3e936b3956342b5422770bd2cb3d9c6fc983eafb Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Sun, 1 Jun 2025 14:31:49 +0100 Subject: [PATCH 5/5] fixup! src: add V8 Fast API for URL.revokeObjectURL skip test on no crypto --- test/parallel/test-url-revokeobjecturl-fast.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/parallel/test-url-revokeobjecturl-fast.js b/test/parallel/test-url-revokeobjecturl-fast.js index 9099c658b8aebe..ec023571831c17 100644 --- a/test/parallel/test-url-revokeobjecturl-fast.js +++ b/test/parallel/test-url-revokeobjecturl-fast.js @@ -5,6 +5,12 @@ const assert = require('node:assert'); const { internalBinding } = require('internal/test/binding'); +// Because registering a Blob URL requires generating a random +// UUID, it can only be done if crypto support is enabled. +if (!common.hasCrypto) { + common.skip('missing crypto'); +} + const blob = new Blob([JSON.stringify({ hello: 'world' }, null, 2)], { type: 'application/json', });