Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] lib: optimize util.format() #215

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions benchmark/util/util-format.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) 2014, Ben Noordhuis <[email protected]>
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
// copyright notice and this permission notice appear in all copies.
//
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

'use strict';

var common = require('../common.js');
var format = require('util').format;

var bench = common.createBenchmark(main, {});

function main(conf) {
var fmts = [];

for (var i = 0; i < 26; i += 1) {
var k = i, fmt = '';
do {
fmt += '%' + String.fromCharCode(97 + k);
k = (k + 1) % 26;
} while (k != i);
fmts.push(fmt);
}

bench.start();

for (var i = 0; i < 1e5; i += 1)
for (var k = 0; k < fmts.length; k += 1)
format(fmts[k]);

bench.end(1e5);
}
3 changes: 3 additions & 0 deletions lib/_linklist.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
function init(list) {
list._idleNext = list;
list._idlePrev = list;
return list;
}
exports.init = init;

Expand Down Expand Up @@ -57,6 +58,7 @@ function remove(item) {

item._idleNext = null;
item._idlePrev = null;
return item;
}
exports.remove = remove;

Expand All @@ -68,6 +70,7 @@ function append(list, item) {
list._idleNext._idlePrev = item;
item._idlePrev = list;
list._idleNext = item;
return item;
}
exports.append = append;

Expand Down
187 changes: 187 additions & 0 deletions lib/sprintf.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
// Copyright (c) 2014, Ben Noordhuis <[email protected]>
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
// copyright notice and this permission notice appear in all copies.
//
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

'use strict';

var L = require('_linklist');
var inspect = require('util').inspect;
var quickeval = process.binding('contextify').quickeval;

function LRU(maxsize) {
this.dict = Object.create(null);
this.list = L.init({});
this.size = 0;
this.maxsize = maxsize;
}

LRU.prototype.find = function(key) {
var val = this.dict[key];

// Move to front of the list unless it's already the first node.
if (val && val !== L.peek(this.list))
L.append(this.list, L.remove(val));

return val;
};

LRU.prototype.insert = function(key, val) {
while (this.size >= this.maxsize) {
delete this.dict[L.shift(this.list).key];
this.size -= 1;
}

var val = L.append(this.list, {key: key});
this.dict[key] = val;
this.size += 1;

return val;
};

function format(fmt /*, ... */) {
return vsprintf(fmt, arguments, compat);
}

function sprintf(fmt /*, ... */) {
return vsprintf(fmt, arguments, extended);
}

function vsprintf(fmt, argv, compile) {
if (typeof(fmt) !== 'string') {
if (compile === compat)
return concat(argv);
fmt = '' + fmt;
}

var lru = compile.lru;
var cached = lru.find(fmt);

if (cached === undefined) {
cached = lru.insert(fmt);
cached.fun = compile(fmt);
}

return cached.fun.apply(null, argv);
}

function concat(argv) {
var argc = argv.length;
var objects = new Array(argc);

for (var i = 0; i < argc; i += 1)
objects[i] = inspect(argv[i]);

return objects.join(' ');
}

// util.format() compatibility mode.
function compat(fmt) {
var index = 0;
var argc = 0;
var body = '';

for (;;) {
var start = index;
index = fmt.indexOf('%', index);
var end = index === -1 ? fmt.length : index;

if (start !== end) {
body += ' s += "' + untaint(fmt.slice(start, end)) + '";';
}

if (index === -1)
break;

index += 1;

var c = fmt[index];
switch (c) {
case 'd':
// %d is really %f; it coerces the argument to a number
// before turning it into a string.
body += ' s += argc > ' + (argc + 1) + ' ? +a' + argc + ' : "%d";';
argc += 1;
index += 1;
continue;

case 'j':
body += ' if (argc > ' + (argc + 1) + ')';
body += ' try { s += JSON.stringify(a' + argc + '); }';
body += ' catch (e) { s += "[Circular]"; }';
body += ' else s += "%j";';
argc += 1;
index += 1;
continue;

case 's':
body += ' s += argc > ' + (argc + 1) + ' ? a' + argc + ' : "%s";';
argc += 1;
index += 1;
continue;

case '"':
c = '\\"';
// Fall through.

default:
body += ' s += "' + c + '";';
index += 1;
continue;
}
}

// The dummy argument lets format() and sprintf() call the formatter
// with .apply() without having to unshift the format argument first.
var source = 'return function(dummy';

for (var i = 0; i < argc; i += 1)
source += ', a' + i;

source += ') { var argc = arguments.length, s = "";' + body;
source += ' for (var i = ' + argc + ' + 1; i < arguments.length; i += 1) {';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you want to use an internal argc instead of arguments.length for for loop as a cached value? Otherwise argc is unused.

source += ' var arg = arguments[i];';
source += ' if (arg === null || typeof(arg) !== "object") s += " " + arg;';
source += ' else s += " " + inspect(arg);';
source += ' }';
source += ' return s;';
source += ' };';

return Function('inspect', source)(inspect);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: the generated code gets optimized as of https://codereview.chromium.org/821553003/

}

function untaint(s) {
return s.replace(/[\\"]|[^\x20-\x7F]/g, function(c) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t /[\\"\r\n\u2028\u2029]/g enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you bring it up: probably not. With Harmony template strings being available now, the generated code is susceptible to template literal injection. I'll have to look at this again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/[\\"\r\n\u2028\u2029]/g is enough, and template strings don’t affect the parsing of double-quoted string literals.

switch (c) {
case '\t': return '\\t';
case '\n': return '\\n';
case '\f': return '\\f';
case '\r': return '\\r';
case '"': return '\\"';
case '\\': return '\\\\';
}
return '\\u' + ('0000' + c.charCodeAt(0).toString(16)).slice(-4);
});
}

compat.lru = new LRU(64);

function extended(fmt) {
throw Error('unimplemented');
}

extended.lru = new LRU(64);

// Hidden export for lib/util.js
Object.defineProperty(sprintf, '_format', {value: format});

module.exports = sprintf;
44 changes: 6 additions & 38 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,13 @@

'use strict';

var formatRegExp = /%[sdj%]/g;
exports.format = function(f) {
if (!isString(f)) {
var objects = [];
for (var i = 0; i < arguments.length; i++) {
objects.push(inspect(arguments[i]));
}
return objects.join(' ');
}

var i = 1;
var args = arguments;
var len = args.length;
var str = String(f).replace(formatRegExp, function(x) {
if (x === '%%') return '%';
if (i >= len) return x;
switch (x) {
case '%s': return String(args[i++]);
case '%d': return Number(args[i++]);
case '%j':
try {
return JSON.stringify(args[i++]);
} catch (_) {
return '[Circular]';
}
default:
return x;
}
});
for (var x = args[i]; i < len; x = args[++i]) {
if (isNull(x) || !isObject(x)) {
str += ' ' + x;
} else {
str += ' ' + inspect(x);
}
}
return str;
};
var formatter;
exports.format = function() {
if (!formatter)
formatter = require('sprintf')._format;

return formatter.apply(null, arguments);
};

// Mark that a method should not be used.
// Returns a modified function which warns once by default.
Expand Down
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
'lib/_stream_duplex.js',
'lib/_stream_transform.js',
'lib/_stream_passthrough.js',
'lib/sprintf.js',
'lib/string_decoder.js',
'lib/sys.js',
'lib/timers.js',
Expand Down