-
Notifications
You must be signed in to change notification settings - Fork 29.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
timers: allow Immediates to be unrefed #18139
Changes from 2 commits
83ff89b
e8d5b4c
a2d51a9
a0313b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -229,6 +229,10 @@ inline uint32_t Environment::ImmediateInfo::count() const { | |
return fields_[kCount]; | ||
} | ||
|
||
inline uint32_t Environment::ImmediateInfo::ref_count() const { | ||
return fields_[kRefCount]; | ||
} | ||
|
||
inline bool Environment::ImmediateInfo::has_outstanding() const { | ||
return fields_[kHasOutstanding] == 1; | ||
} | ||
|
@@ -241,6 +245,14 @@ inline void Environment::ImmediateInfo::count_dec(uint32_t decrement) { | |
fields_[kCount] = fields_[kCount] - decrement; | ||
} | ||
|
||
inline void Environment::ImmediateInfo::ref_count_inc(uint32_t increment) { | ||
fields_[kRefCount] = fields_[kRefCount] + increment; | ||
} | ||
|
||
inline void Environment::ImmediateInfo::ref_count_dec(uint32_t decrement) { | ||
fields_[kRefCount] = fields_[kRefCount] - decrement; | ||
} | ||
|
||
inline Environment::TickInfo::TickInfo(v8::Isolate* isolate) | ||
: fields_(isolate, kFieldsCount) {} | ||
|
||
|
@@ -538,16 +550,27 @@ inline void Environment::set_fs_stats_field_array(double* fields) { | |
|
||
void Environment::SetImmediate(native_immediate_callback cb, | ||
void* data, | ||
v8::Local<v8::Object> obj) { | ||
v8::Local<v8::Object> obj, | ||
bool ref) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want that argument to be used, to be honest (it was solely there to not have to replicate the code within void Environment::CreateImmediate(native_immediate_callback cb,
void* data,
v8::Local<v8::Object> obj,
bool ref) {
native_immediate_callbacks_.push_back({
cb,
data,
std::unique_ptr<v8::Persistent<v8::Object>>(obj.IsEmpty() ?
nullptr : new v8::Persistent<v8::Object>(isolate_, obj)),
ref
});
immediate_info()->count_inc(1);
}
void Environment::SetImmediate(native_immediate_callback cb,
void* data,
v8::Local<v8::Object> obj) {
CreateImmediate(cb, data, obj, true);
if (immediate_info()->ref_count() == 0)
ToggleImmediateRef(true);
immediate_info()->ref_count_inc(1);
}
void Environment::SetUnrefImmediate(native_immediate_callback cb,
void* data,
v8::Local<v8::Object> obj) {
CreateImmediate(cb, data, obj, false);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since |
||
native_immediate_callbacks_.push_back({ | ||
cb, | ||
data, | ||
std::unique_ptr<v8::Persistent<v8::Object>>( | ||
obj.IsEmpty() ? nullptr : new v8::Persistent<v8::Object>(isolate_, obj)) | ||
std::unique_ptr<v8::Persistent<v8::Object>>(obj.IsEmpty() ? | ||
nullptr : new v8::Persistent<v8::Object>(isolate_, obj)), | ||
ref | ||
}); | ||
if (immediate_info()->count() == 0) | ||
ActivateImmediateCheck(); | ||
immediate_info()->count_inc(1); | ||
if (ref) { | ||
if (immediate_info()->ref_count() == 0) | ||
ToggleImmediateRef(true); | ||
immediate_info()->ref_count_inc(1); | ||
} | ||
} | ||
|
||
void Environment::SetUnrefImmediate(native_immediate_callback cb, | ||
void* data, | ||
v8::Local<v8::Object> obj) { | ||
SetImmediate(cb, data, obj, false); | ||
} | ||
|
||
inline performance::performance_state* Environment::performance_state() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a hard time understanding this sentence. Could you re-write it with simpler grammar? 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind rewriting but the current version is basically copy & pasted from Timers. If I'm doing that, I would prefer to do both in the same PR — if you don't mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine then!