Skip to content
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

async_hooks: use typed array stack as fast path #17780

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions lib/internal/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const async_wrap = process.binding('async_wrap');
* It has a fixed size, so if that is exceeded, calls to the native
* side are used instead in pushAsyncIds() and popAsyncIds().
*/
const { async_hook_fields, async_id_fields, async_ids_fast_stack } = async_wrap;
const { async_hook_fields, async_id_fields } = async_wrap;
// Store the pair executionAsyncId and triggerAsyncId in a std::stack on
// Environment::AsyncHooks::ids_stack_ tracks the resource responsible for the
// current execution stack. This is unwound as each resource exits. In the case
Expand Down Expand Up @@ -68,7 +68,7 @@ const active_hooks = {
const { kInit, kBefore, kAfter, kDestroy, kPromiseResolve,
kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId,
kDefaultTriggerAsyncId, kStackLength,
kFastStackCapacity } = async_wrap.constants;
kStackCapacity } = async_wrap.constants;

// Used in AsyncHook and AsyncResource.
const init_symbol = Symbol('init');
Expand Down Expand Up @@ -342,11 +342,11 @@ function emitDestroyScript(asyncId) {
// This is the equivalent of the native push_async_ids() call.
function pushAsyncIds(asyncId, triggerAsyncId) {
const stackLength = async_hook_fields[kStackLength];
if (stackLength >= kFastStackCapacity)
if (stackLength >= async_hook_fields[kStackCapacity])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? You can just check async_ids_stack.length, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… right, yes. :) Done!

return pushAsyncIds_(asyncId, triggerAsyncId);
const offset = stackLength;
async_ids_fast_stack[offset * 2] = async_id_fields[kExecutionAsyncId];
async_ids_fast_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId];
async_wrap.async_ids_stack[offset * 2] = async_id_fields[kExecutionAsyncId];
async_wrap.async_ids_stack[offset * 2 + 1] = async_id_fields[kTriggerAsyncId];
async_hook_fields[kStackLength]++;
async_id_fields[kExecutionAsyncId] = asyncId;
async_id_fields[kTriggerAsyncId] = triggerAsyncId;
Expand All @@ -358,15 +358,15 @@ function popAsyncIds(asyncId) {
if (async_hook_fields[kStackLength] === 0) return false;
const stackLength = async_hook_fields[kStackLength];

if (stackLength > kFastStackCapacity ||
(async_hook_fields[kCheck] > 0 &&
async_id_fields[kExecutionAsyncId] !== asyncId)) {
if (async_hook_fields[kCheck] > 0 &&
async_id_fields[kExecutionAsyncId] !== asyncId) {
// Do the same thing as the native code (i.e. crash hard).
return popAsyncIds_(asyncId);
}

const offset = stackLength - 1;
async_id_fields[kExecutionAsyncId] = async_ids_fast_stack[2 * offset];
async_id_fields[kTriggerAsyncId] = async_ids_fast_stack[2 * offset + 1];
async_id_fields[kExecutionAsyncId] = async_wrap.async_ids_stack[2 * offset];
async_id_fields[kTriggerAsyncId] = async_wrap.async_ids_stack[2 * offset + 1];
async_hook_fields[kStackLength] = offset;
return offset > 0;
}
Expand Down
17 changes: 16 additions & 1 deletion src/aliased_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ class AliasedBuffer {
js_array_.Reset();
}

AliasedBuffer& operator=(AliasedBuffer&& that) {
this->~AliasedBuffer();
isolate_ = that.isolate_;
count_ = that.count_;
byte_offset_ = that.byte_offset_;
buffer_ = that.buffer_;
free_buffer_ = that.free_buffer_;

js_array_.Reset(isolate_, that.js_array_.Get(isolate_));

that.buffer_ = nullptr;
that.js_array_.Reset();
return *this;
}

/**
* Helper class that is returned from operator[] to support assignment into
* a specified location.
Expand Down Expand Up @@ -193,7 +208,7 @@ class AliasedBuffer {
}

private:
v8::Isolate* const isolate_;
v8::Isolate* isolate_;
size_t count_;
size_t byte_offset_;
NativeT* buffer_;
Expand Down
10 changes: 5 additions & 5 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,9 @@ void AsyncWrap::Initialize(Local<Object> target,
"async_id_fields",
env->async_hooks()->async_id_fields().GetJSArray());

FORCE_SET_TARGET_FIELD(target,
"async_ids_fast_stack",
env->async_hooks()->async_ids_fast_stack()
.GetJSArray());
target->Set(context,
env->async_ids_stack_string(),
env->async_hooks()->async_ids_stack().GetJSArray()).FromJust();

Local<Object> constants = Object::New(isolate);
#define SET_HOOKS_CONSTANT(name) \
Expand All @@ -565,7 +564,7 @@ void AsyncWrap::Initialize(Local<Object> target,
SET_HOOKS_CONSTANT(kAsyncIdCounter);
SET_HOOKS_CONSTANT(kDefaultTriggerAsyncId);
SET_HOOKS_CONSTANT(kStackLength);
SET_HOOKS_CONSTANT(kFastStackCapacity);
SET_HOOKS_CONSTANT(kStackCapacity);
#undef SET_HOOKS_CONSTANT
FORCE_SET_TARGET_FIELD(target, "constants", constants);

Expand Down Expand Up @@ -595,6 +594,7 @@ void AsyncWrap::Initialize(Local<Object> target,
env->set_async_hooks_after_function(Local<Function>());
env->set_async_hooks_destroy_function(Local<Function>());
env->set_async_hooks_promise_resolve_function(Local<Function>());
env->set_async_hooks_binding(target);
}


Expand Down
65 changes: 31 additions & 34 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,11 @@ inline MultiIsolatePlatform* IsolateData::platform() const {
return platform_;
}

inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate)
: isolate_(isolate),
async_ids_fast_stack_(isolate, kFastStackCapacity * 2),
fields_(isolate, kFieldsCount),
async_id_fields_(isolate, kUidFieldsCount) {
v8::HandleScope handle_scope(isolate_);
inline Environment::AsyncHooks::AsyncHooks()
: async_ids_stack_(env()->isolate(), 16 * 2),
fields_(env()->isolate(), kFieldsCount),
async_id_fields_(env()->isolate(), kUidFieldsCount) {
v8::HandleScope handle_scope(env()->isolate());

// Always perform async_hooks checks, not just when async_hooks is enabled.
// TODO(AndreasMadsen): Consider removing this for LTS releases.
Expand All @@ -67,6 +66,9 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate)
// and flag changes won't be included.
fields_[kCheck] = 1;

// async_ids_stack_ was initialized to store 16 async_context structs.
fields_[kStackCapacity] = 16;

// kDefaultTriggerAsyncId should be -1, this indicates that there is no
// specified default value and it should fallback to the executionAsyncId.
// 0 is not used as the magic value, because that indicates a missing context
Expand All @@ -82,9 +84,9 @@ inline Environment::AsyncHooks::AsyncHooks(v8::Isolate* isolate)
// strings can be retrieved quickly.
#define V(Provider) \
providers_[AsyncWrap::PROVIDER_ ## Provider].Set( \
isolate_, \
env()->isolate(), \
v8::String::NewFromOneByte( \
isolate_, \
env()->isolate(), \
reinterpret_cast<const uint8_t*>(#Provider), \
v8::NewStringType::kInternalized, \
sizeof(#Provider) - 1).ToLocalChecked());
Expand All @@ -103,19 +105,23 @@ Environment::AsyncHooks::async_id_fields() {
}

inline AliasedBuffer<double, v8::Float64Array>&
Environment::AsyncHooks::async_ids_fast_stack() {
return async_ids_fast_stack_;
Environment::AsyncHooks::async_ids_stack() {
return async_ids_stack_;
}

inline v8::Local<v8::String> Environment::AsyncHooks::provider_string(int idx) {
return providers_[idx].Get(isolate_);
return providers_[idx].Get(env()->isolate());
}

inline void Environment::AsyncHooks::no_force_checks() {
// fields_ does not have the -= operator defined
fields_[kCheck] = fields_[kCheck] - 1;
}

inline Environment* Environment::AsyncHooks::env() {
return Environment::ForAsyncHooks(this);
}

// Remember to keep this code aligned with pushAsyncIds() in JS.
inline void Environment::AsyncHooks::push_async_ids(double async_id,
double trigger_async_id) {
Expand All @@ -127,15 +133,10 @@ inline void Environment::AsyncHooks::push_async_ids(double async_id,
}

uint32_t offset = fields_[kStackLength];
if (offset < kFastStackCapacity) {
async_ids_fast_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId];
async_ids_fast_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId];
} else {
async_ids_stack_.push({
async_id_fields_[kExecutionAsyncId],
async_id_fields_[kTriggerAsyncId]
});
}
if (offset >= fields_[kStackCapacity])
grow_async_ids_stack();
async_ids_stack_[2 * offset] = async_id_fields_[kExecutionAsyncId];
async_ids_stack_[2 * offset + 1] = async_id_fields_[kTriggerAsyncId];
fields_[kStackLength] = fields_[kStackLength] + 1;
async_id_fields_[kExecutionAsyncId] = async_id;
async_id_fields_[kTriggerAsyncId] = trigger_async_id;
Expand All @@ -157,33 +158,25 @@ inline bool Environment::AsyncHooks::pop_async_id(double async_id) {
"actual: %.f, expected: %.f)\n",
async_id_fields_.GetValue(kExecutionAsyncId),
async_id);
Environment* env = Environment::GetCurrent(isolate_);
DumpBacktrace(stderr);
fflush(stderr);
if (!env->abort_on_uncaught_exception())
if (!env()->abort_on_uncaught_exception())
exit(1);
fprintf(stderr, "\n");
fflush(stderr);
ABORT_NO_BACKTRACE();
}

uint32_t offset = fields_[kStackLength] - 1;
if (offset >= kFastStackCapacity) {
auto async_ids = async_ids_stack_.top();
async_ids_stack_.pop();
async_id_fields_[kExecutionAsyncId] = async_ids.async_id;
async_id_fields_[kTriggerAsyncId] = async_ids.trigger_async_id;
} else {
async_id_fields_[kExecutionAsyncId] = async_ids_fast_stack_[2 * offset];
async_id_fields_[kTriggerAsyncId] = async_ids_fast_stack_[2 * offset + 1];
}
CHECK_LT(offset, fields_[kStackCapacity]);
async_id_fields_[kExecutionAsyncId] = async_ids_stack_[2 * offset];
async_id_fields_[kTriggerAsyncId] = async_ids_stack_[2 * offset + 1];
fields_[kStackLength] = offset;

return fields_[kStackLength] > 0;
}

inline void Environment::AsyncHooks::clear_async_id_stack() {
while (!async_ids_stack_.empty())
async_ids_stack_.pop();
async_id_fields_[kExecutionAsyncId] = 0;
async_id_fields_[kTriggerAsyncId] = 0;
fields_[kStackLength] = 0;
Expand All @@ -210,6 +203,11 @@ inline Environment::AsyncHooks::DefaultTriggerAsyncIdScope
}


Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) {
return ContainerOf(&Environment::async_hooks_, hooks);
}


inline Environment::AsyncCallbackScope::AsyncCallbackScope(Environment* env)
: env_(env) {
env_->makecallback_cntr_++;
Expand Down Expand Up @@ -282,7 +280,6 @@ inline Environment::Environment(IsolateData* isolate_data,
v8::Local<v8::Context> context)
: isolate_(context->GetIsolate()),
isolate_data_(isolate_data),
async_hooks_(context->GetIsolate()),
timer_base_(uv_now(isolate_data->event_loop())),
using_domains_(false),
printed_error_(false),
Expand Down
18 changes: 18 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -403,4 +403,22 @@ void Environment::CollectUVExceptionInfo(v8::Local<v8::Value> object,
syscall, message, path, dest);
}


void Environment::AsyncHooks::grow_async_ids_stack() {
const uint32_t old_capacity = fields_[kStackCapacity];
const uint32_t new_capacity = old_capacity * 1.5;
AliasedBuffer<double, v8::Float64Array> new_buffer(
env()->isolate(), new_capacity * 2);

for (uint32_t i = 0; i < old_capacity * 2; ++i)
new_buffer[i] = async_ids_stack_[i];
async_ids_stack_ = std::move(new_buffer);
fields_[kStackCapacity] = new_capacity;

env()->async_hooks_binding()->Set(
env()->context(),
env()->async_ids_stack_string(),
async_ids_stack_.GetJSArray()).FromJust();
}

} // namespace node
27 changes: 13 additions & 14 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class ModuleWrap;
V(address_string, "address") \
V(args_string, "args") \
V(async, "async") \
V(async_ids_stack_string, "async_ids_stack") \
V(buffer_string, "buffer") \
V(bytes_string, "bytes") \
V(bytes_parsed_string, "bytesParsed") \
Expand Down Expand Up @@ -281,6 +282,7 @@ class ModuleWrap;
V(async_hooks_before_function, v8::Function) \
V(async_hooks_after_function, v8::Function) \
V(async_hooks_promise_resolve_function, v8::Function) \
V(async_hooks_binding, v8::Object) \
V(binding_cache_object, v8::Object) \
V(internal_binding_cache_object, v8::Object) \
V(buffer_prototype_object, v8::Object) \
Expand Down Expand Up @@ -370,6 +372,7 @@ class Environment {
kTotals,
kCheck,
kStackLength,
kStackCapacity,
kFieldsCount,
};

Expand All @@ -381,19 +384,14 @@ class Environment {
kUidFieldsCount,
};

enum FastStackFields {
kFastStackCapacity = 64
};

AsyncHooks() = delete;

inline AliasedBuffer<uint32_t, v8::Uint32Array>& fields();
inline AliasedBuffer<double, v8::Float64Array>& async_id_fields();
inline AliasedBuffer<double, v8::Float64Array>& async_ids_fast_stack();
inline AliasedBuffer<double, v8::Float64Array>& async_ids_stack();

inline v8::Local<v8::String> provider_string(int idx);

inline void no_force_checks();
inline Environment* env();

inline void push_async_ids(double async_id, double trigger_async_id);
inline bool pop_async_id(double async_id);
Expand All @@ -418,22 +416,21 @@ class Environment {

private:
friend class Environment; // So we can call the constructor.
inline explicit AsyncHooks(v8::Isolate* isolate);
inline AsyncHooks();
// Keep a list of all Persistent strings used for Provider types.
v8::Eternal<v8::String> providers_[AsyncWrap::PROVIDERS_LENGTH];
// Used by provider_string().
v8::Isolate* isolate_;
// Keep track of the environment copy itself.
Environment* env_;
// Stores the ids of the current execution context stack.
std::stack<async_context> async_ids_stack_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the #include <stack> now, I think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, done!

// Stores the ids of the current execution context stack with limited
// capacity, but with the benefit of much faster access from JS.
AliasedBuffer<double, v8::Float64Array> async_ids_fast_stack_;
AliasedBuffer<double, v8::Float64Array> async_ids_stack_;
// Attached to a Uint32Array that tracks the number of active hooks for
// each type.
AliasedBuffer<uint32_t, v8::Uint32Array> fields_;
// Attached to a Float64Array that tracks the state of async resources.
AliasedBuffer<double, v8::Float64Array> async_id_fields_;

void grow_async_ids_stack();

DISALLOW_COPY_AND_ASSIGN(AsyncHooks);
};

Expand Down Expand Up @@ -689,6 +686,8 @@ class Environment {

inline bool inside_should_not_abort_on_uncaught_scope() const;

static inline Environment* ForAsyncHooks(AsyncHooks* hooks);

private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);
Expand Down