Skip to content

Commit

Permalink
src: slim down env-inl.h
Browse files Browse the repository at this point in the history 
Move big and/or infrequently used functions from env-inl.h to env.cc to
speed up build times and reduce binary bloat.

This commit also touches async_wrap-inl.h and base_object-inl.h because
those are closely interwined with env-inl.h.

Non-functional change.

Refs: #43712

PR-URL: #43745
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
  • Loading branch information
@bnoordhuis @danielleadams
bnoordhuis authored and danielleadams committed Jul 26, 2022
1 parent 364deea commit 8432d65
Show file tree
Hide file tree
Showing 6 changed files with 548 additions and 559 deletions.
7 changes: 0 additions & 7 deletions src/async_wrap-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,6 @@ inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
return MakeCallback(cb_v.As<v8::Function>(), argc, argv);
}


// Defined here to avoid a circular dependency with env-inl.h.
inline AsyncHooks::DefaultTriggerAsyncIdScope ::DefaultTriggerAsyncIdScope(
AsyncWrap* async_wrap)
: DefaultTriggerAsyncIdScope(async_wrap->env(),
async_wrap->get_async_id()) {}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
104 changes: 0 additions & 104 deletions src/base_object-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,40 +32,6 @@

namespace node {

BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
: persistent_handle_(env->isolate(), object), env_(env) {
CHECK_EQ(false, object.IsEmpty());
CHECK_GT(object->InternalFieldCount(), 0);
object->SetAlignedPointerInInternalField(
BaseObject::kSlot,
static_cast<void*>(this));
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
env->modify_base_object_count(1);
}

BaseObject::~BaseObject() {
env()->modify_base_object_count(-1);
env()->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));

if (UNLIKELY(has_pointer_data())) {
PointerData* metadata = pointer_data();
CHECK_EQ(metadata->strong_ptr_count, 0);
metadata->self = nullptr;
if (metadata->weak_ptr_count == 0)
delete metadata;
}

if (persistent_handle_.IsEmpty()) {
// This most likely happened because the weak callback below cleared it.
return;
}

{
v8::HandleScope handle_scope(env()->isolate());
object()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
}
}

void BaseObject::Detach() {
CHECK_GT(pointer_data()->strong_ptr_count, 0);
pointer_data()->is_detached = true;
Expand Down Expand Up @@ -107,28 +73,6 @@ T* BaseObject::FromJSObject(v8::Local<v8::Value> object) {
return static_cast<T*>(FromJSObject(object));
}


void BaseObject::MakeWeak() {
if (has_pointer_data()) {
pointer_data()->wants_weak_jsobj = true;
if (pointer_data()->strong_ptr_count > 0) return;
}

persistent_handle_.SetWeak(
this,
[](const v8::WeakCallbackInfo<BaseObject>& data) {
BaseObject* obj = data.GetParameter();
// Clear the persistent handle so that ~BaseObject() doesn't attempt
// to mess with internal fields, since the JS object may have
// transitioned into an invalid state.
// Refs: https://github.com/nodejs/node/issues/18897
obj->persistent_handle_.Reset();
CHECK_IMPLIES(obj->has_pointer_data(),
obj->pointer_data()->strong_ptr_count == 0);
obj->OnGCCollect();
}, v8::WeakCallbackType::kParameter);
}

void BaseObject::OnGCCollect() {
delete this;
}
Expand All @@ -148,23 +92,6 @@ bool BaseObject::IsWeakOrDetached() const {
return pd->wants_weak_jsobj || pd->is_detached;
}

void BaseObject::LazilyInitializedJSTemplateConstructor(
const v8::FunctionCallbackInfo<v8::Value>& args) {
DCHECK(args.IsConstructCall());
DCHECK_GT(args.This()->InternalFieldCount(), 0);
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
}

v8::Local<v8::FunctionTemplate>
BaseObject::MakeLazilyInitializedJSTemplate(Environment* env) {
v8::Local<v8::FunctionTemplate> t =
env->NewFunctionTemplate(LazilyInitializedJSTemplateConstructor);
t->Inherit(BaseObject::GetConstructorTemplate(env));
t->InstanceTemplate()->SetInternalFieldCount(
BaseObject::kInternalFieldCount);
return t;
}

template <int Field>
void BaseObject::InternalFieldGet(
v8::Local<v8::String> property,
Expand All @@ -185,37 +112,6 @@ bool BaseObject::has_pointer_data() const {
return pointer_data_ != nullptr;
}

BaseObject::PointerData* BaseObject::pointer_data() {
if (!has_pointer_data()) {
PointerData* metadata = new PointerData();
metadata->wants_weak_jsobj = persistent_handle_.IsWeak();
metadata->self = this;
pointer_data_ = metadata;
}
CHECK(has_pointer_data());
return pointer_data_;
}

void BaseObject::decrease_refcount() {
CHECK(has_pointer_data());
PointerData* metadata = pointer_data();
CHECK_GT(metadata->strong_ptr_count, 0);
unsigned int new_refcount = --metadata->strong_ptr_count;
if (new_refcount == 0) {
if (metadata->is_detached) {
OnGCCollect();
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
MakeWeak();
}
}
}

void BaseObject::increase_refcount() {
unsigned int prev_refcount = pointer_data()->strong_ptr_count++;
if (prev_refcount == 0 && !persistent_handle_.IsEmpty())
persistent_handle_.ClearWeak();
}

template <typename T, bool kIsWeak>
BaseObject::PointerData*
BaseObjectPtrImpl<T, kIsWeak>::pointer_data() const {
Expand Down
16 changes: 8 additions & 8 deletions src/base_object.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class BaseObject : public MemoryRetainer {

// Associates this object with `object`. It uses the 0th internal field for
// that, and in particular aborts if there is no such field.
inline BaseObject(Environment* env, v8::Local<v8::Object> object);
inline ~BaseObject() override;
BaseObject(Environment* env, v8::Local<v8::Object> object);
~BaseObject() override;

BaseObject() = delete;

Expand All @@ -65,7 +65,7 @@ class BaseObject : public MemoryRetainer {
// was also passed to the `BaseObject()` constructor initially.
// This may return `nullptr` if the C++ object has not been constructed yet,
// e.g. when the JS object used `MakeLazilyInitializedJSTemplate`.
static inline void LazilyInitializedJSTemplateConstructor(
static void LazilyInitializedJSTemplateConstructor(
const v8::FunctionCallbackInfo<v8::Value>& args);
static inline BaseObject* FromJSObject(v8::Local<v8::Value> object);
template <typename T>
Expand All @@ -74,7 +74,7 @@ class BaseObject : public MemoryRetainer {
// Make the `v8::Global` a weak reference and, `delete` this object once
// the JS object has been garbage collected and there are no (strong)
// BaseObjectPtr references to it.
inline void MakeWeak();
void MakeWeak();

// Undo `MakeWeak()`, i.e. turn this into a strong reference that is a GC
// root and will not be touched by the garbage collector.
Expand All @@ -88,7 +88,7 @@ class BaseObject : public MemoryRetainer {
// Utility to create a FunctionTemplate with one internal field (used for
// the `BaseObject*` pointer) and a constructor that initializes that field
// to `nullptr`.
static inline v8::Local<v8::FunctionTemplate> MakeLazilyInitializedJSTemplate(
static v8::Local<v8::FunctionTemplate> MakeLazilyInitializedJSTemplate(
Environment* env);

// Setter/Getter pair for internal fields that can be passed to SetAccessor.
Expand Down Expand Up @@ -202,11 +202,11 @@ class BaseObject : public MemoryRetainer {
inline bool has_pointer_data() const;
// This creates a PointerData struct if none was associated with this
// BaseObject before.
inline PointerData* pointer_data();
PointerData* pointer_data();

// Functions that adjust the strong pointer count.
inline void decrease_refcount();
inline void increase_refcount();
void decrease_refcount();
void increase_refcount();

Environment* env_;
PointerData* pointer_data_ = nullptr;
Expand Down
Loading

0 comments on commit 8432d65

Please sign in to comment.