Skip to content

Commit a4087c9

Browse files
committed
src: fix inefficient usage of v8_inspector::StringView
v8_inspector::StringView can either be one-byte or two-byte strings. Node has currently two places where it's unconditionally assumed that it's a two-byte StringView. This requires the upstream V8 inspector to unnecessarily create a copy of the string: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/inspector/v8-inspector-session-impl.cc;l=192-199;drc=05bacd38e7a31e92afe0fd66081dfa2cc03b934a This is particularly slow, especially for large CDP messages, as the serialized JSON is iterated 8-bit char by 8-bit char and each one widened to a 16-bit char. This PR introduces a small helper that correctly converts a StringView to a v8::String instance honoring the "is8Bit" flag. This allows upstream V8 to remove the unnecessary widening copy.
1 parent 433bd1b commit a4087c9

File tree

4 files changed

+21
-8
lines changed

4 files changed

+21
-8
lines changed

src/inspector_js_api.cc

+1-3
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,7 @@ class JSBindingsConnection : public BaseObject {
7676
HandleScope handle_scope(isolate);
7777
Context::Scope context_scope(env_->context());
7878
Local<Value> argument;
79-
if (!String::NewFromTwoByte(isolate, message.characters16(),
80-
NewStringType::kNormal,
81-
message.length()).ToLocal(&argument)) return;
79+
if (!StringViewToV8String(isolate, message).ToLocal(&argument)) return;
8280
connection_->OnMessage(argument);
8381
}
8482

src/inspector_profiler.cc

+1-5
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,7 @@ void V8ProfilerConnection::V8ProfilerSessionDelegate::SendMessageToFrontend(
8989
const char* type = connection_->type();
9090
// Convert StringView to a Local<String>.
9191
Local<String> message_str;
92-
if (!String::NewFromTwoByte(isolate,
93-
message.characters16(),
94-
NewStringType::kNormal,
95-
message.length())
96-
.ToLocal(&message_str)) {
92+
if (!StringViewToV8String(isolate, message).ToLocal(&message_str)) {
9793
fprintf(
9894
stderr, "Failed to convert %s profile message to V8 string\n", type);
9995
return;

src/util-inl.h

+14
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,20 @@ inline v8::Local<v8::String> OneByteString(v8::Isolate* isolate,
213213
.ToLocalChecked();
214214
}
215215

216+
v8::MaybeLocal<v8::String> StringViewToV8String(
217+
v8::Isolate* isolate,
218+
v8_inspector::StringView string) {
219+
if (string.is8Bit()) {
220+
return v8::String::NewFromOneByte(isolate,
221+
string.characters8(),
222+
v8::NewStringType::kNormal,
223+
string.length());
224+
}
225+
return v8::String::NewFromTwoByte(isolate, string.characters16(),
226+
v8::NewStringType::kNormal,
227+
string.length());
228+
}
229+
216230
void SwapBytes16(char* data, size_t nbytes) {
217231
CHECK_EQ(nbytes % 2, 0);
218232

src/util.h

+5
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include "uv.h"
2828
#include "v8.h"
29+
#include "v8-inspector.h"
2930

3031
#include "node.h"
3132
#include "node_exit_code.h"
@@ -353,6 +354,10 @@ inline v8::Local<v8::String> FIXED_ONE_BYTE_STRING(
353354
return OneByteString(isolate, arr.data(), N - 1);
354355
}
355356

357+
// Convenience wrapper to handle both one- and two-byte inspector strings.
358+
inline v8::MaybeLocal<v8::String> StringViewToV8String(
359+
v8::Isolate* isolate,
360+
v8_inspector::StringView string);
356361

357362

358363
// Swaps bytes in place. nbytes is the number of bytes to swap and must be a

0 commit comments

Comments
 (0)