From 946315f7d81826f8edd4b9ada87f4285cf88539e Mon Sep 17 00:00:00 2001
From: Trevor Norris <trev.norris@gmail.com>
Date: Tue, 10 Nov 2015 17:04:56 -0700
Subject: [PATCH] node: improve GetActiveHandles performance

Improve performance of process._getActiveHandles by sending handles in
batches to JS to be set on the passed Array. Add test to check proper
active handles are returned.

Alter implementation of GetActiveRequests to match GetActiveHandles'
implementation.

PR-URL: https://github.com/nodejs/node/pull/3780
Reviewed-By: Fedor Indutny <fedor@indutny.com>
---
 src/node.cc                                   | 40 +++++++++--------
 .../parallel/test-process-getactivehandles.js | 44 +++++++++++++++++++
 2 files changed, 66 insertions(+), 18 deletions(-)
 create mode 100644 test/parallel/test-process-getactivehandles.js

diff --git a/src/node.cc b/src/node.cc
index 4c08bae04b..be80ab3124 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -1608,27 +1608,21 @@ static void GetActiveRequests(const FunctionCallbackInfo<Value>& args) {
   Local<Array> ary = Array::New(args.GetIsolate());
   Local<Context> ctx = env->context();
   Local<Function> fn = env->push_values_to_array_function();
-  static const size_t argc = 8;
-  Local<Value> argv[argc];
-  size_t i = 0;
+  Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+  size_t idx = 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>();
-        }
-      }
+    if (w->persistent().IsEmpty())
+      continue;
+    argv[idx] = w->object();
+    if (++idx >= ARRAY_SIZE(argv)) {
+      fn->Call(ctx, ary, idx, argv).ToLocalChecked();
+      idx = 0;
     }
   }
 
-  const size_t remainder = i % argc;
-  if (remainder > 0) {
-    HandleScope scope(env->isolate());
-    fn->Call(ctx, ary, remainder, argv).ToLocalChecked();
+  if (idx > 0) {
+    fn->Call(ctx, ary, idx, argv).ToLocalChecked();
   }
 
   args.GetReturnValue().Set(ary);
@@ -1641,7 +1635,10 @@ void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
   Local<Array> ary = Array::New(env->isolate());
-  int i = 0;
+  Local<Context> ctx = env->context();
+  Local<Function> fn = env->push_values_to_array_function();
+  Local<Value> argv[NODE_PUSH_VAL_TO_ARRAY_MAX];
+  size_t idx = 0;
 
   Local<String> owner_sym = env->owner_string();
 
@@ -1652,7 +1649,14 @@ void GetActiveHandles(const FunctionCallbackInfo<Value>& args) {
     Local<Value> owner = object->Get(owner_sym);
     if (owner->IsUndefined())
       owner = object;
-    ary->Set(i++, owner);
+    argv[idx] = owner;
+    if (++idx >= ARRAY_SIZE(argv)) {
+      fn->Call(ctx, ary, idx, argv).ToLocalChecked();
+      idx = 0;
+    }
+  }
+  if (idx > 0) {
+    fn->Call(ctx, ary, idx, argv).ToLocalChecked();
   }
 
   args.GetReturnValue().Set(ary);
diff --git a/test/parallel/test-process-getactivehandles.js b/test/parallel/test-process-getactivehandles.js
new file mode 100644
index 0000000000..96464cf3b2
--- /dev/null
+++ b/test/parallel/test-process-getactivehandles.js
@@ -0,0 +1,44 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+const NUM = 8;
+const connections = [];
+const clients = [];
+var clients_counter = 0;
+
+const server = net.createServer(function listener(c) {
+  connections.push(c);
+}).listen(common.PORT, function makeConnections() {
+  for (var i = 0; i < NUM; i++) {
+    net.connect(common.PORT, function connected() {
+      clientConnected(this);
+    });
+  }
+});
+
+
+function clientConnected(client) {
+  clients.push(client);
+  if (++clients_counter >= NUM)
+    checkAll();
+}
+
+
+function checkAll() {
+  const handles = process._getActiveHandles();
+
+  clients.forEach(function(item) {
+    assert.ok(handles.indexOf(item) > -1);
+    item.destroy();
+  });
+
+  connections.forEach(function(item) {
+    assert.ok(handles.indexOf(item) > -1);
+    item.end();
+  });
+
+  assert.ok(handles.indexOf(server) > -1);
+  server.close();
+}