v8: optimize getHeapStatistics

Since setting object properties in C++ can be slow, pass data to JS using preallocated smalloc buffer and create object in JS instead. PR-URL: #469 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
nodejs · Jan 21, 2015 · 5435cf2 · trevnorris · Jan 21, 2015 · bnoordhuis
1 parent 5d01463
commit 5435cf2
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 20 deletions.
diff --git a/lib/v8.js b/lib/v8.js
@@ -15,5 +15,29 @@
 'use strict';
 
 var v8binding = process.binding('v8');
-exports.getHeapStatistics = v8binding.getHeapStatistics;
+var smalloc = require('smalloc');
+
+var heapStatisticsBuffer = smalloc.alloc(v8binding.kHeapStatisticsBufferLength,
+                                         v8binding.kHeapStatisticsBufferType);
+
+var kTotalHeapSizeIndex = v8binding.kTotalHeapSizeIndex;
+var kTotalHeapSizeExecutableIndex = v8binding.kTotalHeapSizeExecutableIndex;
+var kTotalPhysicalSizeIndex = v8binding.kTotalPhysicalSizeIndex;
+var kUsedHeapSizeIndex = v8binding.kUsedHeapSizeIndex;
+var kHeapSizeLimitIndex = v8binding.kHeapSizeLimitIndex;
+
+exports.getHeapStatistics = function() {
+  var buffer = heapStatisticsBuffer;
+
+  v8binding.getHeapStatistics(buffer);
+
+  return {
+    'total_heap_size': buffer[kTotalHeapSizeIndex],
+    'total_heap_size_executable': buffer[kTotalHeapSizeExecutableIndex],
+    'total_physical_size': buffer[kTotalPhysicalSizeIndex],
+    'used_heap_size': buffer[kUsedHeapSizeIndex],
+    'heap_size_limit': buffer[kHeapSizeLimitIndex]
+  };
+};
+
 exports.setFlagsFromString = v8binding.setFlagsFromString;
diff --git a/src/env.h b/src/env.h
@@ -92,7 +92,6 @@ namespace node {
   V(fsevent_string, "FSEvent")                                                \
   V(gid_string, "gid")                                                        \
   V(handle_string, "handle")                                                  \
-  V(heap_size_limit_string, "heap_size_limit")                                \
   V(heap_total_string, "heapTotal")                                           \
   V(heap_used_string, "heapUsed")                                             \
   V(hostmaster_string, "hostmaster")                                          \
@@ -198,13 +197,9 @@ namespace node {
   V(tls_sni_string, "tls_sni")                                                \
   V(tls_string, "tls")                                                        \
   V(tls_ticket_string, "tlsTicket")                                           \
-  V(total_heap_size_executable_string, "total_heap_size_executable")          \
-  V(total_heap_size_string, "total_heap_size")                                \
-  V(total_physical_size_string, "total_physical_size")                        \
   V(type_string, "type")                                                      \
   V(uid_string, "uid")                                                        \
   V(unknown_string, "<unknown>")                                              \
-  V(used_heap_size_string, "used_heap_size")                                  \
   V(user_string, "user")                                                      \
   V(uv_string, "uv")                                                          \
   V(valid_from_string, "valid_from")                                          \

diff --git a/src/node_v8.cc b/src/node_v8.cc
@@ -8,6 +8,7 @@
 namespace node {
 
 using v8::Context;
+using v8::ExternalArrayType;
 using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::Handle;
@@ -20,25 +21,41 @@ using v8::Uint32;
 using v8::V8;
 using v8::Value;
 
+#define HEAP_STATISTICS_PROPERTIES(V)                                         \
+  V(0, total_heap_size, kTotalHeapSizeIndex)                                  \
+  V(1, total_heap_size_executable, kTotalHeapSizeExecutableIndex)             \
+  V(2, total_physical_size, kTotalPhysicalSizeIndex)                          \
+  V(3, used_heap_size, kUsedHeapSizeIndex)                                    \
+  V(4, heap_size_limit, kHeapSizeLimitIndex)
+
+#define V(a, b, c) +1
+static const size_t kHeapStatisticsBufferLength = HEAP_STATISTICS_PROPERTIES(V);
+#undef V
+
+static const ExternalArrayType kHeapStatisticsBufferType =
+    v8::kExternalUint32Array;
 
 void GetHeapStatistics(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
+  CHECK(args.Length() == 1 && args[0]->IsObject());
+
   Isolate* isolate = args.GetIsolate();
   HeapStatistics s;
   isolate->GetHeapStatistics(&s);
-  Local<Object> info = Object::New(isolate);
-  // TODO(trevnorris): Setting many object properties in C++ is a significant
-  // performance hit. Redo this to pass the results to JS and create/set the
-  // properties there.
-#define V(name)                                                               \
-  info->Set(env->name ## _string(), Uint32::NewFromUnsigned(isolate, s.name()))
-  V(total_heap_size);
-  V(total_heap_size_executable);
-  V(total_physical_size);
-  V(used_heap_size);
-  V(heap_size_limit);
+  Local<Object> obj = args[0].As<Object>();
+  uint32_t* data =
+      static_cast<uint32_t*>(obj->GetIndexedPropertiesExternalArrayData());
+
+  CHECK_NE(data, nullptr);
+  ASSERT_EQ(obj->GetIndexedPropertiesExternalArrayDataType(),
+            kHeapStatisticsBufferType);
+  ASSERT_EQ(obj->GetIndexedPropertiesExternalArrayDataLength(),
+            kHeapStatisticsBufferLength);
+
+#define V(i, name, _)                                                         \
+  data[i] = static_cast<uint32_t>(s.name());
+
+  HEAP_STATISTICS_PROPERTIES(V)
 #undef V
-  args.GetReturnValue().Set(info);
 }
 
 
@@ -54,6 +71,23 @@ void InitializeV8Bindings(Handle<Object> target,
   Environment* env = Environment::GetCurrent(context);
   env->SetMethod(target, "getHeapStatistics", GetHeapStatistics);
   env->SetMethod(target, "setFlagsFromString", SetFlagsFromString);
+
+  target->Set(FIXED_ONE_BYTE_STRING(env->isolate(),
+                                    "kHeapStatisticsBufferLength"),
+              Uint32::NewFromUnsigned(env->isolate(),
+                                      kHeapStatisticsBufferLength));
+
+  target->Set(FIXED_ONE_BYTE_STRING(env->isolate(),
+                                    "kHeapStatisticsBufferType"),
+              Uint32::NewFromUnsigned(env->isolate(),
+                                      kHeapStatisticsBufferType));
+
+#define V(i, _, name)                                                         \
+  target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #name),                   \
+              Uint32::NewFromUnsigned(env->isolate(), i));
+
+  HEAP_STATISTICS_PROPERTIES(V)
+#undef V
 }
 
 }  // namespace node

diff --git a/src/smalloc.h b/src/smalloc.h
@@ -49,7 +49,7 @@ NODE_EXTERN size_t ExternalArraySize(enum v8::ExternalArrayType type);
  *        v8::kExternalFloatArray);
  *    v8::Local<v8::Object> obj = v8::Object::New();
  *    char* data = static_cast<char*>(malloc(byte_length * array_length));
- *    node::smalloc::Alloc(obj, data, byte_length, v8::kExternalFloatArray);
+ *    node::smalloc::Alloc(env, obj, data, byte_length, v8::kExternalFloatArray);
  *    obj->Set(v8::String::NewFromUtf8("length"),
  *             v8::Integer::NewFromUnsigned(array_length));
  * \code