Skip to content

Commit

Permalink
console: allow Object.prototype fields as labels
Browse files Browse the repository at this point in the history
This is a backport of 6c3647c from
v0.12 to v0.10.

Console.prototype.timeEnd() returns NaN if the timer label
corresponds to a property on Object.prototype. In v0.12, this was fixed
by using Object.create(null) to construct the _times object

However, the version of V8 in the v0.10 branch makes this fix not work
as expected. In v0.10, this commit changes the _times object into a
array of objects of the form:

{ label: someLabel, time: staringWallClockTime }

someLabel can thus be any string, including any string that represents
any Object.prototype field.

Fixes #9116.

PR: #9215
PR-URL: nodejs/node-v0.x-archive#9215
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
Julien Gilli committed Feb 18, 2015
1 parent ad06848 commit c8239c0
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
41 changes: 34 additions & 7 deletions lib/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

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

function Console(stdout, stderr) {
if (!(this instanceof Console)) {
Expand All @@ -40,7 +41,7 @@ function Console(stdout, stderr) {
Object.defineProperty(this, '_stdout', prop);
prop.value = stderr;
Object.defineProperty(this, '_stderr', prop);
prop.value = {};
prop.value = [];
Object.defineProperty(this, '_times', prop);

// bind the prototype functions to this Console instance
Expand Down Expand Up @@ -70,18 +71,44 @@ Console.prototype.dir = function(object) {
};


function findTimeWithLabel(times, label) {
assert(Array.isArray(times), 'times must be an Array');
assert(typeof label === 'string', 'label must be a string');

var found;

times.some(function findTime(item) {
if (item.label === label) {
found = item;
return true;
}
});

return found;
}

Console.prototype.time = function(label) {
this._times[label] = Date.now();
var timeEntry = findTimeWithLabel(this._times, label);
var wallClockTime = Date.now();
if (!timeEntry) {
this._times.push({ label: label, time: wallClockTime });
} else {
timeEntry.time = wallClockTime;
}
};


Console.prototype.timeEnd = function(label) {
var time = this._times[label];
if (!time) {
var wallClockTimeEnd = Date.now();
var timeEntry = findTimeWithLabel(this._times, label);

if (!timeEntry) {
throw new Error('No such label: ' + label);
} else {
assert(typeof timeEntry.time === 'number',
'start time must be a number');
var duration = wallClockTimeEnd - timeEntry.time;
this.log('%s: %dms', label, duration);
}
var duration = Date.now() - time;
this.log('%s: %dms', label, duration);
};


Expand Down
29 changes: 21 additions & 8 deletions test/simple/test-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,22 @@ console.log({slashes: '\\\\'});
console._stderr = process.stdout;
console.trace('This is a %j %d', { formatted: 'trace' }, 10, 'foo');

assert.throws(function () {
console.timeEnd('no such label');
});

assert.doesNotThrow(function () {
console.time('label');
console.timeEnd('label');
});

console.time('__proto__');
console.timeEnd('__proto__');
console.time('constructor');
console.timeEnd('constructor');
console.time('hasOwnProperty');
console.timeEnd('hasOwnProperty');

global.process.stdout.write = stdout_write;

assert.equal('foo\n', strings.shift());
Expand All @@ -55,11 +71,8 @@ assert.equal("{ slashes: '\\\\\\\\' }\n", strings.shift());
assert.equal('Trace: This is a {"formatted":"trace"} 10 foo',
strings.shift().split('\n').shift());

assert.throws(function () {
console.timeEnd('no such label');
});

assert.doesNotThrow(function () {
console.time('label');
console.timeEnd('label');
});
assert.ok(/^label: \d+ms$/.test(strings.shift().trim()));
assert.ok(/^__proto__: \d+ms$/.test(strings.shift().trim()));
assert.ok(/^constructor: \d+ms$/.test(strings.shift().trim()));
assert.ok(/^hasOwnProperty: \d+ms$/.test(strings.shift().trim()));
assert.equal(strings.length, 0);

0 comments on commit c8239c0

Please sign in to comment.