From 41919626e5e3dcc04101a2c21f689fbfad24686f Mon Sep 17 00:00:00 2001 From: Jesus Seijas Date: Fri, 14 Apr 2017 02:51:56 +0200 Subject: [PATCH] util: refactor format method.Performance improved. Method format refactored to make it more maintenable, replacing the switch by a function factory, that returns the appropiated function given the character (d, i , f, j, s). Also, performance when formatting an string that contains several consecutive % symbols is improved. The test: `const numSamples = 10000000; const strPercents = '%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%s%%%%%%%%%%%%%%%%%i%%%%%%%%%%%%%%%%%%%%%%%%%%'; var s; console.time('Percents'); for (let i = 0; i < numSamples; i++) { s = util.format(strPercents, 'test', 12); } console.timeEnd('Percents');` Original time: 28399.708ms After refactor: 23763.788ms Improved: 16% PR-URL: https://github.com/nodejs/node/pull/12407 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Roman Reiss Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Brian White --- lib/util.js | 40 +++++++++++-------------------- test/parallel/test-util-format.js | 8 +++++++ 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/lib/util.js b/lib/util.js index d73b12dcfd59b3..56a8ff68c822db 100644 --- a/lib/util.js +++ b/lib/util.js @@ -68,63 +68,51 @@ exports.format = function(f) { return objects.join(' '); } - var argLen = arguments.length; - - if (argLen === 1) return f; + if (arguments.length === 1) return f; var str = ''; var a = 1; var lastPos = 0; for (var i = 0; i < f.length;) { if (f.charCodeAt(i) === 37/*'%'*/ && i + 1 < f.length) { + if (f.charCodeAt(i + 1) !== 37/*'%'*/ && a >= arguments.length) { + ++i; + continue; + } switch (f.charCodeAt(i + 1)) { case 100: // 'd' - if (a >= argLen) - break; if (lastPos < i) str += f.slice(lastPos, i); str += Number(arguments[a++]); - lastPos = i = i + 2; - continue; + break; case 105: // 'i' - if (a >= argLen) - break; if (lastPos < i) str += f.slice(lastPos, i); str += parseInt(arguments[a++]); - lastPos = i = i + 2; - continue; + break; case 102: // 'f' - if (a >= argLen) - break; if (lastPos < i) str += f.slice(lastPos, i); str += parseFloat(arguments[a++]); - lastPos = i = i + 2; - continue; + break; case 106: // 'j' - if (a >= argLen) - break; if (lastPos < i) str += f.slice(lastPos, i); str += tryStringify(arguments[a++]); - lastPos = i = i + 2; - continue; + break; case 115: // 's' - if (a >= argLen) - break; if (lastPos < i) str += f.slice(lastPos, i); str += String(arguments[a++]); - lastPos = i = i + 2; - continue; + break; case 37: // '%' if (lastPos < i) str += f.slice(lastPos, i); str += '%'; - lastPos = i = i + 2; - continue; + break; } + lastPos = i = i + 2; + continue; } ++i; } @@ -132,7 +120,7 @@ exports.format = function(f) { str = f; else if (lastPos < f.length) str += f.slice(lastPos); - while (a < argLen) { + while (a < arguments.length) { const x = arguments[a++]; if (x === null || (typeof x !== 'object' && typeof x !== 'symbol')) { str += ' ' + x; diff --git a/test/parallel/test-util-format.js b/test/parallel/test-util-format.js index c3b2cc95d1c7ba..93998c598a0e1f 100644 --- a/test/parallel/test-util-format.js +++ b/test/parallel/test-util-format.js @@ -106,6 +106,8 @@ assert.strictEqual(util.format('%%s%s', 'foo'), '%sfoo'); assert.strictEqual(util.format('%s:%s'), '%s:%s'); assert.strictEqual(util.format('%s:%s', undefined), 'undefined:%s'); assert.strictEqual(util.format('%s:%s', 'foo'), 'foo:%s'); +assert.strictEqual(util.format('%s:%i', 'foo'), 'foo:%i'); +assert.strictEqual(util.format('%s:%f', 'foo'), 'foo:%f'); assert.strictEqual(util.format('%s:%s', 'foo', 'bar'), 'foo:bar'); assert.strictEqual(util.format('%s:%s', 'foo', 'bar', 'baz'), 'foo:bar baz'); assert.strictEqual(util.format('%%%s%%', 'hi'), '%hi%'); @@ -114,6 +116,12 @@ assert.strictEqual(util.format('%sbc%%def', 'a'), 'abc%def'); assert.strictEqual(util.format('%d:%d', 12, 30), '12:30'); assert.strictEqual(util.format('%d:%d', 12), '12:%d'); assert.strictEqual(util.format('%d:%d'), '%d:%d'); +assert.strictEqual(util.format('%i:%i', 12, 30), '12:30'); +assert.strictEqual(util.format('%i:%i', 12), '12:%i'); +assert.strictEqual(util.format('%i:%i'), '%i:%i'); +assert.strictEqual(util.format('%f:%f', 12, 30), '12:30'); +assert.strictEqual(util.format('%f:%f', 12), '12:%f'); +assert.strictEqual(util.format('%f:%f'), '%f:%f'); assert.strictEqual(util.format('o: %j, a: %j', {}, []), 'o: {}, a: []'); assert.strictEqual(util.format('o: %j, a: %j', {}), 'o: {}, a: %j'); assert.strictEqual(util.format('o: %j, a: %j'), 'o: %j, a: %j');