Skip to content

Commit 18e16e7

Browse files
jasnelladuh95
jasnell
authored and
aduh95
committed
src: cache permission strings
Use env_property strings for permissions since those are fixed. Avoid creating new string instances each time. Also use ToV8Value for a couple since we're in here. Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #65158 Reviewed-By: Xuguang Mei <meixuguang@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent ac8a68e commit 18e16e7

5 files changed

Lines changed: 81 additions & 23 deletions

File tree

‎src/env-inl.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,14 @@ void Environment::set_process_exit_handler(
841841
#undef VY
842842
#undef VP
843843

844+
#define V(Name, label, _, __) \
845+
inline v8::Local<v8::String> \
846+
IsolateData::Name##_permission_string() const { \
847+
return Name##_permission_string##_.Get(isolate_); \
848+
}
849+
PERMISSIONS(V)
850+
#undef V
851+
844852
#define VM(PropertyName) V(PropertyName##_binding_template, v8::ObjectTemplate)
845853
#define V(PropertyName, TypeName) \
846854
inline v8::Local<TypeName> IsolateData::PropertyName() const { \
@@ -870,6 +878,14 @@ void Environment::set_process_exit_handler(
870878
#undef VY
871879
#undef VP
872880

881+
#define V(Name, label, _, __) \
882+
inline v8::Local<v8::String> \
883+
Environment::Name##_permission_string() const { \
884+
return isolate_data()->Name##_permission_string(); \
885+
}
886+
PERMISSIONS(V)
887+
#undef V
888+
873889
#define V(PropertyName, TypeName) \
874890
inline v8::Local<TypeName> Environment::PropertyName() const { \
875891
return isolate_data()->PropertyName(); \

‎src/env.cc‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,12 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
352352
#undef VS
353353
#undef VP
354354

355+
#define V(Name, label, _, __) \
356+
info.primitive_values.push_back( \
357+
creator->AddData(Name##_permission_string##_.Get(isolate)));
358+
PERMISSIONS(V)
359+
#undef V
360+
355361
info.primitive_values.reserve(info.primitive_values.size() +
356362
AsyncWrap::PROVIDERS_LENGTH);
357363
for (size_t i = 0; i < AsyncWrap::PROVIDERS_LENGTH; i++) {
@@ -411,6 +417,21 @@ void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) {
411417
#undef VS
412418
#undef VP
413419

420+
#define V(Name, label, _, __) \
421+
do { \
422+
MaybeLocal<String> maybe_field = \
423+
isolate_->GetDataFromSnapshotOnce<String>( \
424+
info->primitive_values[i++]); \
425+
Local<String> field; \
426+
if (!maybe_field.ToLocal(&field)) { \
427+
fprintf(stderr, \
428+
"Failed to deserialize " #Name "_permission_string\n"); \
429+
} \
430+
Name##_permission_string##_.Set(isolate_, field); \
431+
} while (0);
432+
PERMISSIONS(V)
433+
#undef V
434+
414435
for (size_t j = 0; j < AsyncWrap::PROVIDERS_LENGTH; j++) {
415436
MaybeLocal<String> maybe_field =
416437
isolate_->GetDataFromSnapshotOnce<String>(info->primitive_values[i++]);
@@ -512,6 +533,17 @@ void IsolateData::CreateProperties() {
512533
PER_ISOLATE_STRING_PROPERTIES(V)
513534
#undef V
514535

536+
#define V(Name, label, _, __) \
537+
Name##_permission_string##_.Set( \
538+
isolate_, \
539+
String::NewFromOneByte(isolate_, \
540+
reinterpret_cast<const uint8_t*>(#Name), \
541+
NewStringType::kInternalized, \
542+
sizeof(#Name) - 1) \
543+
.ToLocalChecked());
544+
PERMISSIONS(V)
545+
#undef V
546+
515547
// Create all the provider strings that will be passed to JS. Place them in
516548
// an array so the array index matches the PROVIDER id offset. This way the
517549
// strings can be retrieved quickly.
@@ -622,6 +654,11 @@ void IsolateData::MemoryInfo(MemoryTracker* tracker) const {
622654
PER_ISOLATE_STRING_PROPERTIES(V)
623655
#undef V
624656

657+
#define V(Name, label, _, __) \
658+
tracker->TrackField(#Name "_permission_string", Name##_permission_string());
659+
PERMISSIONS(V)
660+
#undef V
661+
625662
tracker->TrackField("async_wrap_providers", async_wrap_providers_);
626663

627664
if (node_allocator_ != nullptr) {

‎src/env.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,11 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
188188
#undef VS
189189
#undef VP
190190

191+
#define V(Name, label, _, __) \
192+
inline v8::Local<v8::String> Name##_permission_string() const;
193+
PERMISSIONS(V)
194+
#undef V
195+
191196
#define VM(PropertyName) V(PropertyName##_binding_template, v8::ObjectTemplate)
192197
#define V(PropertyName, TypeName) \
193198
inline v8::Local<TypeName> PropertyName() const; \
@@ -233,6 +238,12 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
233238
#undef VS
234239
#undef VY
235240
#undef VP
241+
242+
#define V(Name, label, _, __) \
243+
v8::Eternal<v8::String> Name##_permission_string##_;
244+
PERMISSIONS(V)
245+
#undef V
246+
236247
// Keep a list of all Persistent strings used for AsyncWrap Provider types.
237248
std::array<v8::Eternal<v8::String>, AsyncWrap::PROVIDERS_LENGTH>
238249
async_wrap_providers_;
@@ -870,6 +881,11 @@ class Environment final : public MemoryRetainer {
870881
#undef VY
871882
#undef VP
872883

884+
#define V(Name, label, _, __) \
885+
inline v8::Local<v8::String> Name##_permission_string() const;
886+
PERMISSIONS(V)
887+
#undef V
888+
873889
#define V(PropertyName, TypeName) \
874890
inline v8::Local<TypeName> PropertyName() const; \
875891
inline void set_ ## PropertyName(v8::Local<TypeName> value);

‎src/permission/permission.cc‎

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,11 @@ static void Has(const FunctionCallbackInfo<Value>& args) {
102102
} // namespace
103103

104104
#define V(Name, label, _, __) \
105-
if (perm == PermissionScope::k##Name) return #Name;
106-
const char* Permission::PermissionToString(const PermissionScope perm) {
105+
if (perm == PermissionScope::k##Name) return env->Name##_permission_string();
106+
v8::Local<v8::String> Permission::PermissionToString(
107+
Environment* env, const PermissionScope perm) {
107108
PERMISSIONS(V)
108-
return nullptr;
109+
UNREACHABLE();
109110
}
110111
#undef V
111112

@@ -178,12 +179,9 @@ MaybeLocal<Value> CreateAccessDeniedError(Environment* env,
178179
Local<Object> err = ERR_ACCESS_DENIED(
179180
env->isolate(), "Access to this API has been restricted. %s", suggestion);
180181

181-
Local<Value> perm_string;
182182
Local<Value> resource_string;
183-
std::string_view perm_str = Permission::PermissionToString(perm);
184-
if (!ToV8Value(env->context(), perm_str, env->isolate())
185-
.ToLocal(&perm_string) ||
186-
!ToV8Value(env->context(), res, env->isolate())
183+
Local<Value> perm_string = Permission::PermissionToString(env, perm);
184+
if (!ToV8Value(env->context(), res, env->isolate())
187185
.ToLocal(&resource_string) ||
188186
err->Set(env->context(), env->permission_string(), perm_string)
189187
.IsNothing() ||
@@ -249,18 +247,13 @@ bool Permission::is_scope_granted(Environment* env,
249247
v8::Local<v8::Context> context = env->context();
250248
v8::Local<v8::Object> msg =
251249
v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
252-
const char* perm_str = PermissionToString(permission);
253250
msg->Set(context,
254251
env->permission_string(),
255-
v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked())
252+
PermissionToString(env, permission))
256253
.Check();
257254
msg->Set(context,
258255
env->resource_string(),
259-
v8::String::NewFromUtf8(isolate,
260-
res.data(),
261-
v8::NewStringType::kNormal,
262-
static_cast<int>(res.size()))
263-
.ToLocalChecked())
256+
ToV8Value(context, res).ToLocalChecked())
264257
.Check();
265258
ch->Publish(env, msg);
266259
publishing_ = false;
@@ -319,18 +312,13 @@ void Permission::Drop(Environment* env,
319312
v8::Local<v8::Context> context = env->context();
320313
v8::Local<v8::Object> msg =
321314
v8::Object::New(isolate, v8::Null(isolate), nullptr, nullptr, 0);
322-
const char* perm_str = PermissionToString(scope);
323315
msg->Set(context,
324316
env->permission_string(),
325-
v8::String::NewFromUtf8(isolate, perm_str).ToLocalChecked())
317+
PermissionToString(env, scope))
326318
.Check();
327319
msg->Set(context,
328320
env->resource_string(),
329-
v8::String::NewFromUtf8(isolate,
330-
param.data(),
331-
v8::NewStringType::kNormal,
332-
static_cast<int>(param.size()))
333-
.ToLocalChecked())
321+
ToV8Value(context, param).ToLocalChecked())
334322
.Check();
335323
msg->Set(context,
336324
FIXED_ONE_BYTE_STRING(isolate, "drop"),

‎src/permission/permission.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ class Permission {
111111
FORCE_INLINE bool warning_only() const { return warning_only_; }
112112

113113
static PermissionScope StringToPermission(const std::string& perm);
114-
static const char* PermissionToString(PermissionScope perm);
114+
static v8::Local<v8::String> PermissionToString(Environment* env,
115+
PermissionScope perm);
115116
static void ThrowAccessDenied(Environment* env,
116117
PermissionScope perm,
117118
const std::string_view& res);

0 commit comments

Comments
 (0)