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

src: speed up process.getActiveResourcesInfo() #46014

Merged
Show file tree
Hide file tree
Changes from all commits
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
45 changes: 45 additions & 0 deletions benchmark/process/getActiveResourcesInfo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';

const { createBenchmark } = require('../common.js');

const { connect, createServer } = require('net');
const { open } = require('fs');

const bench = createBenchmark(main, {
handlesCount: [1e4],
requestsCount: [1e4],
timeoutsCount: [1e4],
immediatesCount: [1e4],
n: [1e5],
});

function main({ handlesCount, requestsCount, timeoutsCount, immediatesCount, n }) {
const server = createServer().listen();
const clients = [];
for (let i = 0; i < handlesCount; i++) {
clients.push(connect({ port: server.address().port }));
}

for (let i = 0; i < requestsCount; i++) {
open(__filename, 'r', () => {});
}

for (let i = 0; i < timeoutsCount; ++i) {
setTimeout(() => {}, 1);
}

for (let i = 0; i < immediatesCount; ++i) {
setImmediate(() => {});
}

bench.start();
for (let i = 0; i < n; ++i) {
process.getActiveResourcesInfo();
}
bench.end(n);

for (const client of clients) {
client.destroy();
}
server.close();
}
13 changes: 1 addition & 12 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@
setupPrepareStackTrace();

const {
Array,
ArrayPrototypeFill,
ArrayPrototypePushApply,
FunctionPrototypeCall,
JSONParse,
Number,
Expand Down Expand Up @@ -171,15 +168,7 @@ const rawMethods = internalBinding('process_methods');
// TODO(joyeecheung): either remove them or make them public
process._getActiveRequests = rawMethods._getActiveRequests;
process._getActiveHandles = rawMethods._getActiveHandles;

process.getActiveResourcesInfo = function() {
const timerCounts = internalTimers.getTimerCounts();
const info = rawMethods._getActiveRequestsInfo();
ArrayPrototypePushApply(info, rawMethods._getActiveHandlesInfo());
ArrayPrototypePushApply(info, ArrayPrototypeFill(new Array(timerCounts.timeoutCount), 'Timeout'));
ArrayPrototypePushApply(info, ArrayPrototypeFill(new Array(timerCounts.immediateCount), 'Immediate'));
return info;
};
process.getActiveResourcesInfo = rawMethods.getActiveResourcesInfo;

// TODO(joyeecheung): remove these
process.reallyExit = rawMethods.reallyExit;
Expand Down
25 changes: 11 additions & 14 deletions lib/internal/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ const {
toggleTimerRef,
getLibuvNow,
immediateInfo,
timeoutInfo,
toggleImmediateRef
} = internalBinding('timers');

Expand Down Expand Up @@ -137,7 +138,11 @@ let timerListId = NumberMIN_SAFE_INTEGER;
const kRefed = Symbol('refed');

let nextExpiry = Infinity;
let refCount = 0;
// timeoutInfo is an Int32Array that contains the reference count of Timeout
// objects at index 0. This is a TypedArray so that GetActiveResourcesInfo() in
// `src/node_process_methods.cc` is able to access this value without crossing
// the JS-C++ boundary, which is slow at the time of writing.
timeoutInfo[0] = 0;
RaisinTen marked this conversation as resolved.
Show resolved Hide resolved

// This is a priority queue with a custom sorting function that first compares
// the expiry times of two lists and if they're the same then compares their
Expand Down Expand Up @@ -302,12 +307,12 @@ class ImmediateList {
const immediateQueue = new ImmediateList();

function incRefCount() {
if (refCount++ === 0)
if (timeoutInfo[0]++ === 0)
toggleTimerRef(true);
}

function decRefCount() {
if (--refCount === 0)
if (--timeoutInfo[0] === 0)
toggleTimerRef(false);
}

Expand Down Expand Up @@ -498,7 +503,7 @@ function getTimerCallbacks(runNextTicks) {
while ((list = timerListQueue.peek()) != null) {
if (list.expiry > now) {
nextExpiry = list.expiry;
return refCount > 0 ? nextExpiry : -nextExpiry;
return timeoutInfo[0] > 0 ? nextExpiry : -nextExpiry;
}
if (ranAtLeastOneList)
runNextTicks();
Expand Down Expand Up @@ -544,7 +549,7 @@ function getTimerCallbacks(runNextTicks) {
timer._destroyed = true;

if (timer[kRefed])
refCount--;
timeoutInfo[0]--;

if (destroyHooksExist())
emitDestroy(asyncId);
Expand Down Expand Up @@ -572,7 +577,7 @@ function getTimerCallbacks(runNextTicks) {
timer._destroyed = true;

if (timer[kRefed])
refCount--;
timeoutInfo[0]--;

if (destroyHooksExist())
emitDestroy(asyncId);
Expand Down Expand Up @@ -643,13 +648,6 @@ class Immediate {
}
}

function getTimerCounts() {
return {
timeoutCount: refCount,
immediateCount: immediateInfo[kRefCount],
};
}

module.exports = {
TIMEOUT_MAX,
kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals.
Expand All @@ -676,5 +674,4 @@ module.exports = {
timerListQueue,
decRefCount,
incRefCount,
getTimerCounts,
};
4 changes: 4 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,10 @@ inline ImmediateInfo* Environment::immediate_info() {
return &immediate_info_;
}

inline AliasedInt32Array& Environment::timeout_info() {
return timeout_info_;
}

inline TickInfo* Environment::tick_info() {
return &tick_info_;
}
Expand Down
4 changes: 4 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ Environment::Environment(IsolateData* isolate_data,
isolate_data_(isolate_data),
async_hooks_(isolate, MAYBE_FIELD_PTR(env_info, async_hooks)),
immediate_info_(isolate, MAYBE_FIELD_PTR(env_info, immediate_info)),
timeout_info_(isolate_, 1, MAYBE_FIELD_PTR(env_info, timeout_info)),
tick_info_(isolate, MAYBE_FIELD_PTR(env_info, tick_info)),
timer_base_(uv_now(isolate_data->event_loop())),
exec_argv_(exec_args),
Expand Down Expand Up @@ -1607,6 +1608,7 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {

info.async_hooks = async_hooks_.Serialize(ctx, creator);
info.immediate_info = immediate_info_.Serialize(ctx, creator);
info.timeout_info = timeout_info_.Serialize(ctx, creator);
info.tick_info = tick_info_.Serialize(ctx, creator);
info.performance_state = performance_state_->Serialize(ctx, creator);
info.exit_info = exit_info_.Serialize(ctx, creator);
Expand Down Expand Up @@ -1653,6 +1655,7 @@ void Environment::DeserializeProperties(const EnvSerializeInfo* info) {
builtins_in_snapshot = info->builtins;
async_hooks_.Deserialize(ctx);
immediate_info_.Deserialize(ctx);
timeout_info_.Deserialize(ctx);
tick_info_.Deserialize(ctx);
performance_state_->Deserialize(ctx);
exit_info_.Deserialize(ctx);
Expand Down Expand Up @@ -1852,6 +1855,7 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("cleanup_queue", cleanup_queue_);
tracker->TrackField("async_hooks", async_hooks_);
tracker->TrackField("immediate_info", immediate_info_);
tracker->TrackField("timeout_info", timeout_info_);
tracker->TrackField("tick_info", tick_info_);
tracker->TrackField("principal_realm", principal_realm_);

Expand Down
3 changes: 3 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ struct EnvSerializeInfo {
AsyncHooks::SerializeInfo async_hooks;
TickInfo::SerializeInfo tick_info;
ImmediateInfo::SerializeInfo immediate_info;
AliasedBufferIndex timeout_info;
performance::PerformanceState::SerializeInfo performance_state;
AliasedBufferIndex exit_info;
AliasedBufferIndex stream_base_state;
Expand Down Expand Up @@ -672,6 +673,7 @@ class Environment : public MemoryRetainer {

inline AsyncHooks* async_hooks();
inline ImmediateInfo* immediate_info();
inline AliasedInt32Array& timeout_info();
inline TickInfo* tick_info();
inline uint64_t timer_base() const;
inline std::shared_ptr<KVStore> env_vars();
Expand Down Expand Up @@ -1010,6 +1012,7 @@ class Environment : public MemoryRetainer {

AsyncHooks async_hooks_;
ImmediateInfo immediate_info_;
AliasedInt32Array timeout_info_;
TickInfo tick_info_;
const uint64_t timer_base_;
std::shared_ptr<KVStore> env_vars_;
Expand Down
50 changes: 26 additions & 24 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,21 +259,6 @@ static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {
Array::New(env->isolate(), request_v.data(), request_v.size()));
}

static void GetActiveRequestsInfo(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

std::vector<Local<Value>> requests_info;
for (ReqWrapBase* req_wrap : *env->req_wrap_queue()) {
AsyncWrap* w = req_wrap->GetAsyncWrap();
if (w->persistent().IsEmpty()) continue;
requests_info.emplace_back(OneByteString(env->isolate(),
w->MemoryInfoName().c_str()));
}

args.GetReturnValue().Set(
Array::New(env->isolate(), requests_info.data(), requests_info.size()));
}

// Non-static, friend of HandleWrap. Could have been a HandleWrap method but
// implemented here for consistency with GetActiveRequests().
void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
Expand All @@ -289,18 +274,37 @@ void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
Array::New(env->isolate(), handle_v.data(), handle_v.size()));
}

void GetActiveHandlesInfo(const FunctionCallbackInfo<Value>& args) {
static void GetActiveResourcesInfo(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
std::vector<Local<Value>> resources_info;
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if you can omit using std::vector for better performance in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestions on how you think that can be done?


// Active requests
for (ReqWrapBase* req_wrap : *env->req_wrap_queue()) {
AsyncWrap* w = req_wrap->GetAsyncWrap();
if (w->persistent().IsEmpty()) continue;
resources_info.emplace_back(
OneByteString(env->isolate(), w->MemoryInfoName().c_str()));
}

std::vector<Local<Value>> handles_info;
// Active handles
for (HandleWrap* w : *env->handle_wrap_queue()) {
if (w->persistent().IsEmpty() || !HandleWrap::HasRef(w)) continue;
handles_info.emplace_back(OneByteString(env->isolate(),
w->MemoryInfoName().c_str()));
resources_info.emplace_back(
OneByteString(env->isolate(), w->MemoryInfoName().c_str()));
}

// Active timeouts
resources_info.insert(resources_info.end(),
env->timeout_info()[0],
OneByteString(env->isolate(), "Timeout"));

// Active immediates
resources_info.insert(resources_info.end(),
env->immediate_info()->ref_count(),
OneByteString(env->isolate(), "Immediate"));

args.GetReturnValue().Set(
Array::New(env->isolate(), handles_info.data(), handles_info.size()));
Array::New(env->isolate(), resources_info.data(), resources_info.size()));
}

static void ResourceUsage(const FunctionCallbackInfo<Value>& args) {
Expand Down Expand Up @@ -583,10 +587,9 @@ static void Initialize(Local<Object> target,
SetMethod(context, target, "resourceUsage", ResourceUsage);

SetMethod(context, target, "_debugEnd", DebugEnd);
SetMethod(context, target, "_getActiveRequestsInfo", GetActiveRequestsInfo);
SetMethod(context, target, "_getActiveRequests", GetActiveRequests);
SetMethod(context, target, "_getActiveHandles", GetActiveHandles);
SetMethod(context, target, "_getActiveHandlesInfo", GetActiveHandlesInfo);
SetMethod(context, target, "getActiveResourcesInfo", GetActiveResourcesInfo);
SetMethod(context, target, "_kill", Kill);
SetMethod(context, target, "_rawDebug", RawDebug);

Expand Down Expand Up @@ -614,9 +617,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(ResourceUsage);

registry->Register(GetActiveRequests);
registry->Register(GetActiveRequestsInfo);
registry->Register(GetActiveHandles);
registry->Register(GetActiveHandlesInfo);
registry->Register(GetActiveResourcesInfo);
registry->Register(Kill);

registry->Register(Cwd);
Expand Down
3 changes: 3 additions & 0 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
<< "// -- async_hooks ends --\n"
<< i.tick_info << ", // tick_info\n"
<< i.immediate_info << ", // immediate_info\n"
<< i.timeout_info << ", // timeout_info\n"
<< "// -- performance_state begins --\n"
<< i.performance_state << ",\n"
<< "// -- performance_state ends --\n"
Expand Down Expand Up @@ -734,6 +735,7 @@ EnvSerializeInfo FileReader::Read() {
result.async_hooks = Read<AsyncHooks::SerializeInfo>();
result.tick_info = Read<TickInfo::SerializeInfo>();
result.immediate_info = Read<ImmediateInfo::SerializeInfo>();
result.timeout_info = Read<AliasedBufferIndex>();
result.performance_state =
Read<performance::PerformanceState::SerializeInfo>();
result.exit_info = Read<AliasedBufferIndex>();
Expand All @@ -755,6 +757,7 @@ size_t FileWriter::Write(const EnvSerializeInfo& data) {
written_total += Write<AsyncHooks::SerializeInfo>(data.async_hooks);
written_total += Write<TickInfo::SerializeInfo>(data.tick_info);
written_total += Write<ImmediateInfo::SerializeInfo>(data.immediate_info);
written_total += Write<AliasedBufferIndex>(data.timeout_info);
written_total += Write<performance::PerformanceState::SerializeInfo>(
data.performance_state);
written_total += Write<AliasedBufferIndex>(data.exit_info);
Expand Down
6 changes: 6 additions & 0 deletions src/timers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ void Initialize(Local<Object> target,
FIXED_ONE_BYTE_STRING(env->isolate(), "immediateInfo"),
env->immediate_info()->fields().GetJSArray())
.Check();

target
->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "timeoutInfo"),
env->timeout_info().GetJSArray())
.Check();
}
} // anonymous namespace
void RegisterTimerExternalReferences(ExternalReferenceRegistry* registry) {
Expand Down