Speed up variable resolution ~10x on node 1.6+

Matt Loring · Matt Loring · commit d9f86a5f275d · 2016-01-15T09:47:57.000-08:00
Use a different mirror resolution path on node 1.6+ which is substantially faster. Continue to use the old implementation on earlier node versions as the new implementation segfaults before node 1.6. Addresses #71
diff --git a/lib/state.js b/lib/state.js
@@ -22,6 +22,7 @@ module.exports = {
 };
 
 var assert = require('assert');
+var semver = require('semver');
 
 var StatusMessage = require('./apiclasses.js').StatusMessage;
 
@@ -345,10 +346,18 @@ StateResolver.prototype.storeObjectToVariableTable_ = function(obj) {
   return idx;
 };
 
-
 StateResolver.prototype.resolveMirror_ = function(mirror) {
-  // See the commented out version of this method below.
-  //
+  if (semver.satisfies(process.version, '<1.6')) {
+    return this.resolveMirrorSlow_(mirror);
+  } else {
+    return this.resolveMirrorFast_(mirror);
+  }
+};
+
+// A slower implementation of resolveMirror_ which is safe for all node versions
+//
+// See https://github.com/iojs/io.js/issues/1190.
+StateResolver.prototype.resolveMirrorSlow_ = function(mirror) {
   // Instead, let's use Object.keys. This will only get the enumerable
   // properties. The other alternative would be Object.getOwnPropertyNames, but
   // I'm going with the former as that's what util.inspect does.
@@ -367,24 +376,23 @@ StateResolver.prototype.resolveMirror_ = function(mirror) {
   };
 };
 
-// Ideally we would use Mirror.properties() method to acquire the properties
-// However, because of a bug that exists in iojs (uptil 1.6.?) and node (still)
-// we can end up with segfaults on objects with interceptors and accessors.
-// See https://github.com/iojs/io.js/issues/1190.
+// A faster implementation of resolveMirror_ which segfaults in node <1.6
 //
-// StateResolver.prototype.resolveMirror_ = function(mirror) {
-//   var members = this.getMirrorProperties_(mirror).map(
-//       this.resolveMirrorProperty_.bind(this));
-//   return {
-//     value: mirror.toText(),
-//     members: members
-//   };
-// };
-// StateResolver.prototype.getMirrorProperties_ = function(mirror) {
-//   var namedProperties = mirror.properties(1, 100); // Limited to 100.
-//   var indexedProperties = mirror.properties(2, 100); // Limited to 100.
-//   return namedProperties.concat(indexedProperties);
-// };
-// StateResolver.prototype.resolveMirrorProperty_ = function(property) {
-//   return this.resolveVariable_(property.name(), property.value());
-// };
+// See https://github.com/iojs/io.js/issues/1190.
+StateResolver.prototype.resolveMirrorFast_ = function(mirror) {
+  var members = this.getMirrorProperties_(mirror).map(
+      this.resolveMirrorProperty_.bind(this));
+  return {
+    value: mirror.toText(),
+    members: members
+  };
+};
+StateResolver.prototype.getMirrorProperties_ = function(mirror) {
+  var numProperties = this.config_.capture.maxProperties || 1000;
+  var namedProperties = mirror.properties(1, numProperties);
+  var indexedProperties = mirror.properties(2, numProperties);
+  return namedProperties.concat(indexedProperties);
+};
+StateResolver.prototype.resolveMirrorProperty_ = function(property) {
+  return this.resolveVariable_(property.name(), property.value());
+};
