Skip to content

Commit 17a5349

Browse files
committed
src: make additional cleanups in node locks impl
* Track memory held by the Lock instance * Clean up some Utf8/TwoByteString handling
1 parent 2992a0f commit 17a5349

File tree

3 files changed

+38
-28
lines changed

3 files changed

+38
-28
lines changed

src/node_locks.cc

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@ using v8::Isolate;
2323
using v8::Local;
2424
using v8::LocalVector;
2525
using v8::MaybeLocal;
26-
using v8::NewStringType;
2726
using v8::Object;
2827
using v8::ObjectTemplate;
2928
using v8::Promise;
3029
using v8::PropertyAttribute;
31-
using v8::String;
3230
using v8::Value;
3331

3432
// Reject two promises and return `false` on failure.
@@ -51,19 +49,26 @@ Lock::Lock(Environment* env,
5149
released_promise_.Reset(env_->isolate(), released);
5250
}
5351

52+
void Lock::MemoryInfo(node::MemoryTracker* tracker) const {
53+
tracker->TrackFieldWithSize("name", name_.size());
54+
tracker->TrackField("client_id", client_id_);
55+
tracker->TrackField("waiting_promise", waiting_promise_);
56+
tracker->TrackField("released_promise", released_promise_);
57+
}
58+
5459
LockRequest::LockRequest(Environment* env,
5560
Local<Promise::Resolver> waiting,
5661
Local<Promise::Resolver> released,
5762
Local<Function> callback,
5863
const std::u16string& name,
5964
Lock::Mode mode,
60-
const std::string& client_id,
65+
std::string client_id,
6166
bool steal,
6267
bool if_available)
6368
: env_(env),
6469
name_(name),
6570
mode_(mode),
66-
client_id_(client_id),
71+
client_id_(std::move(client_id)),
6772
steal_(steal),
6873
if_available_(if_available) {
6974
waiting_promise_.Reset(env_->isolate(), waiting);
@@ -573,22 +578,13 @@ void LockManager::Request(const FunctionCallbackInfo<Value>& args) {
573578
CHECK(args[4]->IsBoolean()); // ifAvailable
574579
CHECK(args[5]->IsFunction()); // callback
575580

576-
Local<String> resource_name_str = args[0].As<String>();
577-
TwoByteValue resource_name_utf16(isolate, resource_name_str);
578-
std::u16string resource_name(
579-
reinterpret_cast<const char16_t*>(*resource_name_utf16),
580-
resource_name_utf16.length());
581-
String::Utf8Value client_id_utf8(isolate, args[1]);
582-
std::string client_id(*client_id_utf8);
583-
String::Utf8Value mode_utf8(isolate, args[2]);
584-
std::string mode_str(*mode_utf8);
581+
TwoByteValue resource_name(isolate, args[0]);
582+
Utf8Value client_id(isolate, args[1]);
583+
Utf8Value mode(isolate, args[2]);
585584
bool steal = args[3]->BooleanValue(isolate);
586585
bool if_available = args[4]->BooleanValue(isolate);
587586
Local<Function> callback = args[5].As<Function>();
588587

589-
Lock::Mode lock_mode =
590-
mode_str == "shared" ? Lock::Mode::Shared : Lock::Mode::Exclusive;
591-
592588
Local<Promise::Resolver> waiting_promise;
593589
Local<Promise::Resolver> released_promise;
594590

@@ -611,15 +607,17 @@ void LockManager::Request(const FunctionCallbackInfo<Value>& args) {
611607
env->AddCleanupHook(LockManager::OnEnvironmentCleanup, env);
612608
}
613609

614-
auto lock_request = std::make_unique<LockRequest>(env,
615-
waiting_promise,
616-
released_promise,
617-
callback,
618-
resource_name,
619-
lock_mode,
620-
client_id,
621-
steal,
622-
if_available);
610+
auto lock_request = std::make_unique<LockRequest>(
611+
env,
612+
waiting_promise,
613+
released_promise,
614+
callback,
615+
resource_name.ToU16String(),
616+
mode.ToStringView() == "shared" ? Lock::Mode::Shared
617+
: Lock::Mode::Exclusive,
618+
client_id.ToString(),
619+
steal,
620+
if_available);
623621
// Steal requests get priority by going to front of queue
624622
if (steal) {
625623
manager->pending_queue_.emplace_front(std::move(lock_request));
@@ -842,6 +840,10 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
842840
registry->Register(OnIfAvailableReject);
843841
}
844842

843+
void LockHolder::MemoryInfo(node::MemoryTracker* tracker) const {
844+
tracker->TrackField("lock", lock_);
845+
}
846+
845847
BaseObjectPtr<LockHolder> LockHolder::Create(Environment* env,
846848
std::shared_ptr<Lock> lock) {
847849
Local<Object> obj;

src/node_locks.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
namespace node::worker::locks {
1717

18-
class Lock final {
18+
class Lock final : public MemoryRetainer {
1919
public:
2020
enum class Mode { Shared, Exclusive };
2121

@@ -53,6 +53,10 @@ class Lock final {
5353
return released_promise_.Get(env_->isolate());
5454
}
5555

56+
void MemoryInfo(node::MemoryTracker* tracker) const override;
57+
SET_MEMORY_INFO_NAME(Lock)
58+
SET_SELF_SIZE(Lock)
59+
5660
private:
5761
Environment* env_;
5862
std::u16string name_;
@@ -79,7 +83,7 @@ class LockHolder final : public BaseObject {
7983

8084
std::shared_ptr<Lock> lock() const { return lock_; }
8185

82-
SET_NO_MEMORY_INFO()
86+
void MemoryInfo(node::MemoryTracker* tracker) const override;
8387
SET_MEMORY_INFO_NAME(LockHolder)
8488
SET_SELF_SIZE(LockHolder)
8589

@@ -101,7 +105,7 @@ class LockRequest final {
101105
v8::Local<v8::Function> callback,
102106
const std::u16string& name,
103107
Lock::Mode mode,
104-
const std::string& client_id,
108+
std::string client_id,
105109
bool steal,
106110
bool if_available);
107111
~LockRequest() = default;

src/util.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,10 @@ class Utf8Value : public MaybeStackBuffer<char> {
562562
class TwoByteValue : public MaybeStackBuffer<uint16_t> {
563563
public:
564564
explicit TwoByteValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
565+
566+
inline std::u16string ToU16String() const {
567+
return std::u16string(reinterpret_cast<const char16_t*>(out()), length());
568+
}
565569
};
566570

567571
class BufferValue : public MaybeStackBuffer<char> {

0 commit comments

Comments
 (0)