From 41f5a29cca5266f0d1d879fce5bc06db66297028 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Tue, 3 Jan 2023 16:06:25 +0530 Subject: [PATCH] src: speed up process.getActiveResourcesInfo() 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: https://github.com/nodejs/node/pull/44445#pullrequestreview-1220052837 Signed-off-by: Darshan Sen PR-URL: https://github.com/nodejs/node/pull/46014 Reviewed-By: Antoine du Hamel Reviewed-By: Chengzhong Wu --- benchmark/process/getActiveResourcesInfo.js | 45 +++++++++++++++++++ lib/internal/bootstrap/node.js | 13 +----- lib/internal/timers.js | 25 +++++------ src/env-inl.h | 4 ++ src/env.cc | 4 ++ src/env.h | 3 ++ src/node_process_methods.cc | 50 +++++++++++---------- src/node_snapshotable.cc | 3 ++ src/timers.cc | 6 +++ 9 files changed, 103 insertions(+), 50 deletions(-) create mode 100644 benchmark/process/getActiveResourcesInfo.js diff --git a/benchmark/process/getActiveResourcesInfo.js b/benchmark/process/getActiveResourcesInfo.js new file mode 100644 index 00000000000000..b62d979ee504f6 --- /dev/null +++ b/benchmark/process/getActiveResourcesInfo.js @@ -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(); +} diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 9d781636d8dc2c..81b441a554e345 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -55,9 +55,6 @@ setupPrepareStackTrace(); const { - Array, - ArrayPrototypeFill, - ArrayPrototypePushApply, FunctionPrototypeCall, JSONParse, ObjectDefineProperty, @@ -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; diff --git a/lib/internal/timers.js b/lib/internal/timers.js index a2a1c1e387bf6a..0f8bdef9dfeb8b 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -87,6 +87,7 @@ const { toggleTimerRef, getLibuvNow, immediateInfo, + timeoutInfo, toggleImmediateRef } = internalBinding('timers'); @@ -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 @@ -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); } @@ -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(); @@ -544,7 +549,7 @@ function getTimerCallbacks(runNextTicks) { timer._destroyed = true; if (timer[kRefed]) - refCount--; + timeoutInfo[0]--; if (destroyHooksExist()) emitDestroy(asyncId); @@ -572,7 +577,7 @@ function getTimerCallbacks(runNextTicks) { timer._destroyed = true; if (timer[kRefed]) - refCount--; + timeoutInfo[0]--; if (destroyHooksExist()) emitDestroy(asyncId); @@ -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. @@ -676,5 +674,4 @@ module.exports = { timerListQueue, decRefCount, incRefCount, - getTimerCounts, }; diff --git a/src/env-inl.h b/src/env-inl.h index fbc4fbb27b065d..65710f7670c0ac 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -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_; } diff --git a/src/env.cc b/src/env.cc index 724035fe17cb30..1f4f7e2017e4c3 100644 --- a/src/env.cc +++ b/src/env.cc @@ -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), @@ -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); @@ -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); @@ -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_); diff --git a/src/env.h b/src/env.h index 4d5e72f9f44762..bcbaf761914c95 100644 --- a/src/env.h +++ b/src/env.h @@ -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; @@ -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 env_vars(); @@ -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 env_vars_; diff --git a/src/node_process_methods.cc b/src/node_process_methods.cc index 296d5245cd5658..29fce1dd0bb476 100644 --- a/src/node_process_methods.cc +++ b/src/node_process_methods.cc @@ -258,21 +258,6 @@ static void GetActiveRequests(const FunctionCallbackInfo& args) { Array::New(env->isolate(), request_v.data(), request_v.size())); } -static void GetActiveRequestsInfo(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - - std::vector> 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& args) { @@ -288,18 +273,37 @@ void GetActiveHandles(const FunctionCallbackInfo& args) { Array::New(env->isolate(), handle_v.data(), handle_v.size())); } -void GetActiveHandlesInfo(const FunctionCallbackInfo& args) { +static void GetActiveResourcesInfo(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); + std::vector> 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> 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& args) { @@ -578,10 +582,9 @@ static void Initialize(Local 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); @@ -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); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 093572ece3d758..41c284119013e0 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -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" @@ -734,6 +735,7 @@ EnvSerializeInfo FileReader::Read() { result.async_hooks = Read(); result.tick_info = Read(); result.immediate_info = Read(); + result.timeout_info = Read(); result.performance_state = Read(); result.exiting = Read(); @@ -755,6 +757,7 @@ size_t FileWriter::Write(const EnvSerializeInfo& data) { written_total += Write(data.async_hooks); written_total += Write(data.tick_info); written_total += Write(data.immediate_info); + written_total += Write(data.timeout_info); written_total += Write( data.performance_state); written_total += Write(data.exiting); diff --git a/src/timers.cc b/src/timers.cc index 2ecfd25f7eeb8b..02aba02c5b73ee 100644 --- a/src/timers.cc +++ b/src/timers.cc @@ -59,6 +59,12 @@ void Initialize(Local 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) {