Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Commit

Permalink
Speed up variable resolution ~10x on node 1.6+
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Matt Loring committed Jan 15, 2016
1 parent 2ba5bbd commit d9f86a5
Showing 1 changed file with 31 additions and 23 deletions.
54 changes: 31 additions & 23 deletions lib/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ module.exports = {
};

var assert = require('assert');
var semver = require('semver');

var StatusMessage = require('./apiclasses.js').StatusMessage;

Expand Down Expand Up @@ -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.
Expand All @@ -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());
};

0 comments on commit d9f86a5

Please sign in to comment.