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

path: refactor for performance and consistency #1778

Closed
wants to merge 2 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
31 changes: 31 additions & 0 deletions benchmark/path/format.js
Original file line number Diff line number Diff line change
@@ -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);
}
27 changes: 27 additions & 0 deletions benchmark/path/isAbsolute.js
Original file line number Diff line number Diff line change
@@ -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]);
}
}
18 changes: 18 additions & 0 deletions benchmark/path/join.js
Original file line number Diff line number Diff line change
@@ -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);
}
18 changes: 18 additions & 0 deletions benchmark/path/normalize.js
Original file line number Diff line number Diff line change
@@ -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);
}
26 changes: 26 additions & 0 deletions benchmark/path/relative.js
Original file line number Diff line number Diff line change
@@ -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');
}
18 changes: 18 additions & 0 deletions benchmark/path/resolve.js
Original file line number Diff line number Diff line change
@@ -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);
}
139 changes: 69 additions & 70 deletions lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if returning the original array and a new array based on conditions is a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a little helper function. There's no point in making a copy of an array when the original is all you need.

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


Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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('/');
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Loading