From 84e4f5b75408790bbf43b1260ace11eb0485b7f6 Mon Sep 17 00:00:00 2001 From: Nathan Woltman Date: Sat, 23 May 2015 00:42:12 -0400 Subject: [PATCH 1/2] path: refactor for performance and consistency Improve performance by: + Not leaking the `arguments` object! + Getting the last character of a string by index, instead of with `.substr()` or `.slice()` Improve code consistency by: + Using `[]` instead of `.charAt()` where possible + Using a function declaration instead of a var declaration + Using `.slice()` with clearer arguments + Checking if `dir` is truthy in `win32.format` (added tests for this) Improve both by: + Making the reusable `trimArray()` function + Standardizing getting certain path statistics with the new `win32StatPath()` function --- lib/path.js | 139 ++++++++++++------------ test/parallel/test-path-parse-format.js | 26 ++++- 2 files changed, 91 insertions(+), 74 deletions(-) diff --git a/lib/path.js b/lib/path.js index b7e28b22250791..69e7992c0139d7 100644 --- a/lib/path.js +++ b/lib/path.js @@ -37,6 +37,29 @@ function normalizeArray(parts, allowAboveRoot) { return res; } +// Returns an array with empty elements removed from either end of the input +// array or the original array if no elements need to be removed +function trimArray(arr) { + var lastIndex = arr.length - 1; + var start = 0; + for (; start <= lastIndex; start++) { + if (arr[start]) + break; + } + + var end = lastIndex; + for (; end >= 0; end--) { + if (arr[end]) + break; + } + + if (start === 0 && end === lastIndex) + return arr; + if (start > end) + return []; + return arr.slice(start, end + 1); +} + // Regex to split a windows path into three parts: [*, device, slash, // tail] windows-only const splitDeviceRe = @@ -62,9 +85,21 @@ function win32SplitPath(filename) { return [device, dir, basename, ext]; } -var normalizeUNCRoot = function(device) { +function win32StatPath(path) { + var result = splitDeviceRe.exec(path), + device = result[1] || '', + isUnc = !!device && device[1] !== ':'; + return { + device, + isUnc, + isAbsolute: isUnc || !!result[2], // UNC paths are always absolute + tail: result[3] + }; +} + +function normalizeUNCRoot(device) { return '\\\\' + device.replace(/^[\\\/]+/, '').replace(/[\\\/]+/g, '\\'); -}; +} // path.resolve([from ...], to) win32.resolve = function() { @@ -99,11 +134,11 @@ win32.resolve = function() { continue; } - var result = splitDeviceRe.exec(path), - device = result[1] || '', - isUnc = device && device.charAt(1) !== ':', - isAbsolute = win32.isAbsolute(path), - tail = result[3]; + var result = win32StatPath(path), + device = result.device, + isUnc = result.isUnc, + isAbsolute = result.isAbsolute, + tail = result.tail; if (device && resolvedDevice && @@ -147,11 +182,11 @@ win32.resolve = function() { win32.normalize = function(path) { assertPath(path); - var result = splitDeviceRe.exec(path), - device = result[1] || '', - isUnc = device && device.charAt(1) !== ':', - isAbsolute = win32.isAbsolute(path), - tail = result[3], + var result = win32StatPath(path), + device = result.device, + isUnc = result.isUnc, + isAbsolute = result.isAbsolute, + tail = result.tail, trailingSlash = /[\\\/]$/.test(tail); // Normalize the tail path @@ -176,23 +211,19 @@ win32.normalize = function(path) { win32.isAbsolute = function(path) { assertPath(path); - - var result = splitDeviceRe.exec(path), - device = result[1] || '', - isUnc = !!device && device.charAt(1) !== ':'; - // UNC paths are always absolute - return !!result[2] || isUnc; + return win32StatPath(path).isAbsolute; }; win32.join = function() { - function f(p) { - if (typeof p !== 'string') { - throw new TypeError('Arguments to path.join must be strings'); + var paths = []; + for (var i = 0; i < arguments.length; i++) { + var arg = arguments[i]; + assertPath(arg); + if (arg) { + paths.push(arg); } - return p; } - var paths = Array.prototype.filter.call(arguments, f); var joined = paths.join('\\'); // Make sure that the joined path doesn't start with two slashes, because @@ -232,25 +263,10 @@ win32.relative = function(from, to) { var lowerFrom = from.toLowerCase(); var lowerTo = to.toLowerCase(); - function trim(arr) { - var start = 0; - for (; start < arr.length; start++) { - if (arr[start] !== '') break; - } - - var end = arr.length - 1; - for (; end >= 0; end--) { - if (arr[end] !== '') break; - } - - if (start > end) return []; - return arr.slice(start, end + 1); - } + var toParts = trimArray(to.split('\\')); - var toParts = trim(to.split('\\')); - - var lowerFromParts = trim(lowerFrom.split('\\')); - var lowerToParts = trim(lowerTo.split('\\')); + var lowerFromParts = trimArray(lowerFrom.split('\\')); + var lowerToParts = trimArray(lowerTo.split('\\')); var length = Math.min(lowerFromParts.length, lowerToParts.length); var samePartsLength = length; @@ -356,15 +372,13 @@ win32.format = function(pathObject) { var dir = pathObject.dir; var base = pathObject.base || ''; - if (dir.slice(dir.length - 1, dir.length) === win32.sep) { - return dir + base; + if (!dir) { + return base; } - - if (dir) { - return dir + win32.sep + base; + if (dir[dir.length - 1] === win32.sep) { + return dir + base; } - - return base; + return dir + win32.sep + base; }; @@ -377,7 +391,7 @@ win32.parse = function(pathString) { } return { root: allParts[0], - dir: allParts[0] + allParts[1].slice(0, allParts[1].length - 1), + dir: allParts[0] + allParts[1].slice(0, -1), base: allParts[2], ext: allParts[3], name: allParts[2].slice(0, allParts[2].length - allParts[3].length) @@ -418,7 +432,7 @@ posix.resolve = function() { } resolvedPath = path + '/' + resolvedPath; - resolvedAbsolute = path.charAt(0) === '/'; + resolvedAbsolute = path[0] === '/'; } // At this point the path should be resolved to a full absolute path, but @@ -437,7 +451,7 @@ posix.normalize = function(path) { assertPath(path); var isAbsolute = posix.isAbsolute(path), - trailingSlash = path.substr(-1) === '/'; + trailingSlash = path && path[path.length - 1] === '/'; // Normalize the path path = normalizeArray(path.split('/'), !isAbsolute).join('/'); @@ -455,7 +469,7 @@ posix.normalize = function(path) { // posix version posix.isAbsolute = function(path) { assertPath(path); - return path.charAt(0) === '/'; + return !!path && path[0] === '/'; }; // posix version @@ -487,23 +501,8 @@ posix.relative = function(from, to) { from = posix.resolve(from).substr(1); to = posix.resolve(to).substr(1); - function trim(arr) { - var start = 0; - for (; start < arr.length; start++) { - if (arr[start] !== '') break; - } - - var end = arr.length - 1; - for (; end >= 0; end--) { - if (arr[end] !== '') break; - } - - if (start > end) return []; - return arr.slice(start, end + 1); - } - - var fromParts = trim(from.split('/')); - var toParts = trim(to.split('/')); + var fromParts = trimArray(from.split('/')); + var toParts = trimArray(to.split('/')); var length = Math.min(fromParts.length, toParts.length); var samePartsLength = length; @@ -602,7 +601,7 @@ posix.parse = function(pathString) { return { root: allParts[0], - dir: allParts[0] + allParts[1].slice(0, allParts[1].length - 1), + dir: allParts[0] + allParts[1].slice(0, -1), base: allParts[2], ext: allParts[3], name: allParts[2].slice(0, allParts[2].length - allParts[3].length) diff --git a/test/parallel/test-path-parse-format.js b/test/parallel/test-path-parse-format.js index 37f37fc9b57c17..677bf3241f0bae 100644 --- a/test/parallel/test-path-parse-format.js +++ b/test/parallel/test-path-parse-format.js @@ -15,7 +15,12 @@ var winPaths = [ '\\\\server two\\shared folder\\file path.zip', '\\\\teela\\admin$\\system32', '\\\\?\\UNC\\server\\share' +]; +var winSpecialCaseFormatTests = [ + [{dir: 'some\\dir'}, 'some\\dir\\'], + [{base: 'index.html'}, 'index.html'], + [{}, ''] ]; var unixPaths = [ @@ -30,6 +35,12 @@ var unixPaths = [ 'C:\\foo' ]; +var unixSpecialCaseFormatTests = [ + [{dir: 'some/dir'}, 'some/dir/'], + [{base: 'index.html'}, 'index.html'], + [{}, ''] +]; + var errors = [ {method: 'parse', input: [null], message: /Path must be a string. Received null/}, @@ -57,10 +68,12 @@ var errors = [ message: /'pathObject.root' must be a string or undefined, not number/}, ]; -check(path.win32, winPaths); -check(path.posix, unixPaths); +checkParseFormat(path.win32, winPaths); +checkParseFormat(path.posix, unixPaths); checkErrors(path.win32); checkErrors(path.posix); +checkFormat(path.win32, winSpecialCaseFormatTests); +checkFormat(path.posix, unixSpecialCaseFormatTests); function checkErrors(path) { errors.forEach(function(errorCase) { @@ -79,8 +92,7 @@ function checkErrors(path) { }); } - -function check(path, paths) { +function checkParseFormat(path, paths) { paths.forEach(function(element, index, array) { var output = path.parse(element); assert.strictEqual(path.format(output), element); @@ -89,3 +101,9 @@ function check(path, paths) { assert.strictEqual(output.ext, path.extname(element)); }); } + +function checkFormat(path, testCases) { + testCases.forEach(function(testCase) { + assert.strictEqual(path.format(testCase[0]), testCase[1]); + }); +} From 363335c6727ab30e3b4a0fa5002c28d5b5922f83 Mon Sep 17 00:00:00 2001 From: Nathan Woltman Date: Fri, 5 Jun 2015 20:43:48 -0400 Subject: [PATCH 2/2] benchmark: Add some path benchmarks for #1778 Path functions being benchmarked are: * format * isAbsolute * join * normalize * relative * resolve --- benchmark/path/format.js | 31 +++++++++++++++++++++++++++++++ benchmark/path/isAbsolute.js | 27 +++++++++++++++++++++++++++ benchmark/path/join.js | 18 ++++++++++++++++++ benchmark/path/normalize.js | 18 ++++++++++++++++++ benchmark/path/relative.js | 26 ++++++++++++++++++++++++++ benchmark/path/resolve.js | 18 ++++++++++++++++++ 6 files changed, 138 insertions(+) create mode 100644 benchmark/path/format.js create mode 100644 benchmark/path/isAbsolute.js create mode 100644 benchmark/path/join.js create mode 100644 benchmark/path/normalize.js create mode 100644 benchmark/path/relative.js create mode 100644 benchmark/path/resolve.js diff --git a/benchmark/path/format.js b/benchmark/path/format.js new file mode 100644 index 00000000000000..02fb691fe7fd07 --- /dev/null +++ b/benchmark/path/format.js @@ -0,0 +1,31 @@ +var common = require('../common.js'); +var path = require('path'); + +var bench = common.createBenchmark(main, { + type: ['win32', 'posix'], + n: [1e7], +}); + +function main(conf) { + var n = +conf.n; + var p = path[conf.type]; + var test = conf.type === 'win32' ? { + root: 'C:\\', + dir: 'C:\\path\\dir', + base: 'index.html', + ext: '.html', + name: 'index' + } : { + root : '/', + dir : '/home/user/dir', + base : 'index.html', + ext : '.html', + name : 'index' + }; + + bench.start(); + for (var i = 0; i < n; i++) { + p.format(test); + } + bench.end(n); +} diff --git a/benchmark/path/isAbsolute.js b/benchmark/path/isAbsolute.js new file mode 100644 index 00000000000000..c9489fe85cbe4d --- /dev/null +++ b/benchmark/path/isAbsolute.js @@ -0,0 +1,27 @@ +var common = require('../common.js'); +var path = require('path'); + +var bench = common.createBenchmark(main, { + type: ['win32', 'posix'], + n: [1e6], +}); + +function main(conf) { + var n = +conf.n; + var p = path[conf.type]; + var tests = conf.type === 'win32' + ? ['//server', 'C:\\baz\\..', 'bar\\baz', '.'] + : ['/foo/bar', '/baz/..', 'bar/baz', '.']; + + bench.start(); + for (var i = 0; i < n; i++) { + runTests(p, tests); + } + bench.end(n); +} + +function runTests(p, tests) { + for (var i = 0; i < tests.length; i++) { + p.isAbsolute(tests[i]); + } +} diff --git a/benchmark/path/join.js b/benchmark/path/join.js new file mode 100644 index 00000000000000..54d02a6450e7be --- /dev/null +++ b/benchmark/path/join.js @@ -0,0 +1,18 @@ +var common = require('../common.js'); +var path = require('path'); + +var bench = common.createBenchmark(main, { + type: ['win32', 'posix'], + n: [1e6], +}); + +function main(conf) { + var n = +conf.n; + var p = path[conf.type]; + + bench.start(); + for (var i = 0; i < n; i++) { + p.join('/foo', 'bar', '', 'baz/asdf', 'quux', '..'); + } + bench.end(n); +} diff --git a/benchmark/path/normalize.js b/benchmark/path/normalize.js new file mode 100644 index 00000000000000..10ca23037a7279 --- /dev/null +++ b/benchmark/path/normalize.js @@ -0,0 +1,18 @@ +var common = require('../common.js'); +var path = require('path'); + +var bench = common.createBenchmark(main, { + type: ['win32', 'posix'], + n: [1e6], +}); + +function main(conf) { + var n = +conf.n; + var p = path[conf.type]; + + bench.start(); + for (var i = 0; i < n; i++) { + p.normalize('/foo/bar//baz/asdf/quux/..'); + } + bench.end(n); +} diff --git a/benchmark/path/relative.js b/benchmark/path/relative.js new file mode 100644 index 00000000000000..3e12f8b532dac3 --- /dev/null +++ b/benchmark/path/relative.js @@ -0,0 +1,26 @@ +var common = require('../common.js'); +var path = require('path'); + +var bench = common.createBenchmark(main, { + type: ['win32', 'posix'], + n: [1e5], +}); + +function main(conf) { + var n = +conf.n; + var runTest = conf.type === 'win32' ? runWin32Test : runPosixTest; + + bench.start(); + for (var i = 0; i < n; i++) { + runTest(); + } + bench.end(n); +} + +function runWin32Test() { + path.win32.relative('C:\\orandea\\test\\aaa', 'C:\\orandea\\impl\\bbb'); +} + +function runPosixTest() { + path.posix.relative('/data/orandea/test/aaa', '/data/orandea/impl/bbb'); +} diff --git a/benchmark/path/resolve.js b/benchmark/path/resolve.js new file mode 100644 index 00000000000000..ecf30f32fab81f --- /dev/null +++ b/benchmark/path/resolve.js @@ -0,0 +1,18 @@ +var common = require('../common.js'); +var path = require('path'); + +var bench = common.createBenchmark(main, { + type: ['win32', 'posix'], + n: [1e6], +}); + +function main(conf) { + var n = +conf.n; + var p = path[conf.type]; + + bench.start(); + for (var i = 0; i < n; i++) { + p.resolve('foo/bar', '/tmp/file/', '..', 'a/../subfile'); + } + bench.end(n); +}