Skip to content

Commit

Permalink
node: improve GetActiveRequests performance
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
trevnorris authored and rvagg committed Oct 22, 2015
1 parent 97d0817 commit ae19617
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ namespace node {
V(zero_return_string, "ZERO_RETURN") \

#define ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) \
V(add_properties_by_index_function, v8::Function) \
V(as_external, v8::External) \
V(async_hooks_init_function, v8::Function) \
V(async_hooks_pre_function, v8::Function) \
Expand Down
39 changes: 35 additions & 4 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,17 @@ void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
}


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

CHECK(args[0]->IsFunction());

env->set_add_properties_by_index_function(args[0].As<Function>());
env->process_object()->Delete(
FIXED_ONE_BYTE_STRING(env->isolate(), "_setupProcessObject"));
}


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

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

Local<Array> ary = Array::New(args.GetIsolate());
int i = 0;
Local<Context> ctx = env->context();
Local<Function> fn = env->add_properties_by_index_function();
static const size_t argc = 8;
Local<Value> argv[argc];
size_t i = 0;

for (auto w : *env->req_wrap_queue()) {
if (w->persistent().IsEmpty() == false) {
argv[i++ % argc] = w->object();
if ((i % argc) == 0) {
HandleScope scope(env->isolate());
fn->Call(ctx, ary, argc, argv).ToLocalChecked();
for (auto&& arg : argv) {
arg = Local<Value>();
}
}
}
}

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

args.GetReturnValue().Set(ary);
}
Expand Down Expand Up @@ -2979,6 +3009,7 @@ void SetupProcessObject(Environment* env,
env->SetMethod(process, "binding", Binding);
env->SetMethod(process, "_linkedBinding", LinkedBinding);

env->SetMethod(process, "_setupProcessObject", SetupProcessObject);
env->SetMethod(process, "_setupNextTick", SetupNextTick);
env->SetMethod(process, "_setupPromises", SetupPromises);
env->SetMethod(process, "_setupDomainUse", SetupDomainUse);
Expand Down
11 changes: 11 additions & 0 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@

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

startup.setupProcessObject();

// do this good and early, since it handles errors.
startup.processFatal();

Expand Down Expand Up @@ -173,6 +175,15 @@
}
}

startup.setupProcessObject = function() {
process._setupProcessObject(setPropByIndex);

function setPropByIndex() {
for (var i = 0; i < arguments.length; i++)
this.push(arguments[i]);
}
};

startup.globalVariables = function() {
global.process = process;
global.global = global;
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-process-getactiverequests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const fs = require('fs');

for (let i = 0; i < 12; i++)
fs.open(__filename, 'r', function() { });

assert.equal(12, process._getActiveRequests().length);

0 comments on commit ae19617

Please sign in to comment.