FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

node: improve GetActiveRequests performance · nodejs/node@ae19617 · GitHub

/ node Public

Commit ae19617

Browse files
trevnorris
authored and
rvagg
committed
node: improve GetActiveRequests performance
v8 is faster at setting object properties in JS than C++. Even when it requires calling into JS from native code. Make process._getActiveRequests() faster by doing this when populating the array containing request objects. Simple benchmark: for (let i = 0; i < 22; i++) fs.open(__filename, 'r', function() { }); let t = process.hrtime(); for (let i = 0; i < 1e6; i++) process._getActiveRequests(); t = process.hrtime(t); console.log((t[0] * 1e9 + t[1]) / 1e6); Results between the two: Previous: 4406 ns/op Patched: 690 ns/op 5.4x faster PR-URL: #3375 Reviewed-By: James Snell <jasnell@gmail.com> Reviewed-By: Ben Noordhuis <ben@strongloop.com>
1 parent 97d0817 commit ae19617

4 files changed

Lines changed: 57 additions & 4 deletions

File tree

‎src/env.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@ namespace node {
227227
V(zero_return_string, "ZERO_RETURN") \
228228

229229
#define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \
230+
V(add_properties_by_index_function, v8::Function) \
230231
V(as_external, v8::External) \
231232
V(async_hooks_init_function, v8::Function) \
232233
V(async_hooks_pre_function, v8::Function) \

‎src/node.cc‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,6 +1009,17 @@ void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
10091009
}
10101010

10111011

1012+
void SetupProcessObject(const FunctionCallbackInfo<Value>& args) {
1013+
Environment* env = Environment::GetCurrent(args);
1014+
1015+
CHECK(args[0]->IsFunction());
1016+
1017+
env->set_add_properties_by_index_function(args[0].As<Function>());
1018+
env->process_object()->Delete(
1019+
FIXED_ONE_BYTE_STRING(env->isolate(), "_setupProcessObject"));
1020+
}
1021+
1022+
10121023
void SetupNextTick(const FunctionCallbackInfo<Value>& args) {
10131024
Environment* env = Environment::GetCurrent(args);
10141025

@@ -1546,11 +1557,30 @@ static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {
15461557
Environment* env = Environment::GetCurrent(args);
15471558

15481559
Local<Array> ary = Array::New(args.GetIsolate());
1549-
int i = 0;
1560+
Local<Context> ctx = env->context();
1561+
Local<Function> fn = env->add_properties_by_index_function();
1562+
static const size_t argc = 8;
1563+
Local<Value> argv[argc];
1564+
size_t i = 0;
1565+
1566+
for (auto w : *env->req_wrap_queue()) {
1567+
if (w->persistent().IsEmpty() == false) {
1568+
argv[i++ % argc] = w->object();
1569+
if ((i % argc) == 0) {
1570+
HandleScope scope(env->isolate());
1571+
fn->Call(ctx, ary, argc, argv).ToLocalChecked();
1572+
for (auto&& arg : argv) {
1573+
arg = Local<Value>();
1574+
}
1575+
}
1576+
}
1577+
}
15501578

1551-
for (auto w : *env->req_wrap_queue())
1552-
if (w->persistent().IsEmpty() == false)
1553-
ary->Set(i++, w->object());
1579+
const size_t remainder = i % argc;
1580+
if (remainder > 0) {
1581+
HandleScope scope(env->isolate());
1582+
fn->Call(ctx, ary, remainder, argv).ToLocalChecked();
1583+
}
15541584

15551585
args.GetReturnValue().Set(ary);
15561586
}
@@ -2979,6 +3009,7 @@ void SetupProcessObject(Environment* env,
29793009
env->SetMethod(process, "binding", Binding);
29803010
env->SetMethod(process, "_linkedBinding", LinkedBinding);
29813011

3012+
env->SetMethod(process, "_setupProcessObject", SetupProcessObject);
29823013
env->SetMethod(process, "_setupNextTick", SetupNextTick);
29833014
env->SetMethod(process, "_setupPromises", SetupPromises);
29843015
env->SetMethod(process, "_setupDomainUse", SetupDomainUse);

‎src/node.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
process.EventEmitter = EventEmitter; // process.EventEmitter is deprecated
2424

25+
startup.setupProcessObject();
26+
2527
// do this good and early, since it handles errors.
2628
startup.processFatal();
2729

@@ -173,6 +175,15 @@
173175
}
174176
}
175177

178+
startup.setupProcessObject = function() {
179+
process._setupProcessObject(setPropByIndex);
180+
181+
function setPropByIndex() {
182+
for (var i = 0; i < arguments.length; i++)
183+
this.push(arguments[i]);
184+
}
185+
};
186+
176187
startup.globalVariables = function() {
177188
global.process = process;
178189
global.global = global;

‎test/parallel/test-process-getactiverequests.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
const fs = require('fs');
6+
7+
for (let i = 0; i < 12; i++)
8+
fs.open(__filename, 'r', function() { });
9+
10+
assert.equal(12, process._getActiveRequests().length);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL