Skip to content

Commit

Permalink
src: speed up process.getActiveResourcesInfo()
Browse files Browse the repository at this point in the history
This change reduces the number of calls that were crossing the JS-C++
boundary to 1 and also removes the need for calling Array::New()
multiple times internally and ArrayPrototypeConcat-ing the results
later on, thus improving performance.

Refs: #44445 (review)
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: #46014
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
  • Loading branch information
RaisinTen authored and juanarbol committed Jan 31, 2023
1 parent 8de6425 commit 41f5a29
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 50 deletions.
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,
ObjectDefineProperty,
Expand Down Expand Up @@ -192,15 +189,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;

// 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 @@ -649,6 +649,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 @@ -1603,6 +1604,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.exiting = exiting_.Serialize(ctx, creator);
Expand Down Expand Up @@ -1649,6 +1651,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);
exiting_.Deserialize(ctx);
Expand Down Expand Up @@ -1845,6 +1848,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 @@ -460,6 +460,7 @@ struct EnvSerializeInfo {
AsyncHooks::SerializeInfo async_hooks;
TickInfo::SerializeInfo tick_info;
ImmediateInfo::SerializeInfo immediate_info;
AliasedBufferIndex timeout_info;
performance::PerformanceState::SerializeInfo performance_state;
AliasedBufferIndex exiting;
AliasedBufferIndex stream_base_state;
Expand Down Expand Up @@ -667,6 +668,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 @@ -988,6 +990,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 @@ -258,21 +258,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 @@ -288,18 +273,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;

// 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 @@ -578,10 +582,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 @@ -609,9 +612,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.exiting = 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.exiting);
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

0 comments on commit 41f5a29

Please sign in to comment.