Merge pull request #104 from pahen/perf

Improve performance on large codebase

lib/tree.js

@@ -1,12 +1,19 @@
 'use strict';
 
+const os = require('os');
 const fs = require('mz/fs');
 const path = require('path');
 const commondir = require('commondir');
 const walk = require('walkdir');
 const dependencyTree = require('dependency-tree');
 const log = require('./log');
 
+/**
+ * Check if running on Windows.
+ * @type {Boolean}
+ */
+const isWin = (os.platform() === 'win32');
+
 class Tree {
 	/**
 	 * Class constructor.
@@ -30,47 +37,17 @@ class Tree {
 	}
 
 	/**
-	 * Generate the tree from the given files
-	 * @param  {Array} files
-	 * @return {Object}
+	 * Set the base directory (compute the common one if multiple).
+	 * @param {Array} dirs
 	 */
-	generateTree(files) {
-		const depTree = {};
-		const visited = {};
-
-		files.forEach((file) => {
-			if (visited[file]) {
-				return;
-			}
-
-			Object.assign(depTree, dependencyTree({
-				filename: file,
-				directory: this.baseDir,
-				requireConfig: this.config.requireConfig,
-				webpackConfig: this.config.webpackConfig,
-				visited: visited,
-				filter: this.filterPath.bind(this)
-			}));
-		});
-
-		let tree = this.flatten(depTree);
-
-		if (this.config.excludeRegExp) {
-			tree = this.exclude(tree, this.config.excludeRegExp);
+	setBaseDir(dirs) {
+		if (this.config.baseDir) {
+			this.baseDir = path.resolve(this.config.baseDir);
+		} else {
+			this.baseDir = commondir(dirs);
 		}
 
-		tree = this.sort(tree);
-
-		return tree;
-	}
-
-	/**
-	 * Filter out some paths from found files
-	 * @param  {String} path
-	 * @return {Boolean}
-	 */
-	filterPath(path) {
-		return this.config.includeNpm || path.indexOf('node_modules') < 0;
+		log('using base directory %s', this.baseDir);
 	}
 
 	/**
@@ -120,61 +97,99 @@ class Tree {
 	}
 
 	/**
-	 * Set the base directory (compute the common one if multiple).
-	 * @param {Array} dirs
+	 * Generate the tree from the given files
+	 * @param  {Array} files
+	 * @return {Object}
 	 */
-	setBaseDir(dirs) {
-		if (this.config.baseDir) {
-			this.baseDir = path.resolve(this.config.baseDir);
-		} else {
-			this.baseDir = commondir(dirs);
+	generateTree(files) {
+		const depTree = {};
+		const visited = {};
+
+		files.forEach((file) => {
+			if (visited[file]) {
+				return;
+			}
+
+			Object.assign(depTree, dependencyTree({
+				filename: file,
+				directory: this.baseDir,
+				requireConfig: this.config.requireConfig,
+				webpackConfig: this.config.webpackConfig,
+				visited: visited,
+				filter: this.filterPath.bind(this)
+			}));
+		});
+
+		let tree = this.convertTree(depTree, {}, {});
+
+		if (this.config.excludeRegExp) {
+			tree = this.exclude(tree, this.config.excludeRegExp);
 		}
 
-		log('using base directory %s', this.baseDir);
-	}
+		tree = this.sort(tree);
 
+		return tree;
+	}
 
 	/**
-	 * Flatten deep tree produced by `dependency-tree`.
-	 * @param  {Object} deepTree
-	 * @param  {Object} [tree]
+	 * Convert deep tree produced by dependency-tree to a
+	 * shallow (one level deep) tree used by madge.
+	 * @param  {Object} depTree
+	 * @param  {Object} tree
+	 * @param  {Object} pathCache
 	 * @return {Object}
 	 */
-	flatten(deepTree, tree) {
-		tree = tree || {};
-
-		Object
-			.keys(deepTree)
-			.forEach((key) => {
-				const id = this.processPath(key);
-
-				if (!tree[id]) {
-					tree[id] = Object
-						.keys(deepTree[key])
-						.map((dep) => this.processPath(dep));
+	convertTree(depTree, tree, pathCache) {
+		for (const key in depTree) {
+			const id = this.processPath(key, pathCache);
+
+			if (!tree[id]) {
+				tree[id] = [];
+
+				for (const dep in depTree[key]) {
+					tree[id].push(this.processPath(dep, pathCache));
 				}
+			}
 
-				this.flatten(deepTree[key], tree);
-			});
+			this.convertTree(depTree[key], tree, pathCache);
+		}
 
 		return tree;
 	}
 
 	/**
-	 * Process path.
+	 * Process absolute path and return a shorter one.
 	 * @param  {String} absPath
+	 * @param  {Object} cache
 	 * @return {String}
 	 */
-	processPath(absPath) {
-		absPath = path.relative(this.baseDir, absPath);
+	processPath(absPath, cache) {
+		if (cache[absPath]) {
+			return cache[absPath];
+		}
+
+		let relPath = path.relative(this.baseDir, absPath);
 
 		if (!this.config.showFileExtension) {
-			absPath = absPath.replace(/\.\w+$/, '');
+			relPath = relPath.substr(0, relPath.lastIndexOf('.'));
+		}
+
+		if (isWin) {
+			relPath = relPath.replace(/\\/g, '/');
 		}
 
-		absPath = absPath.replace(/\\/g, '/');
+		cache[absPath] = relPath;
 
-		return absPath;
+		return relPath;
+	}
+
+	/**
+	 * Filter out some paths from found files
+	 * @param  {String} path
+	 * @return {Boolean}
+	 */
+	filterPath(path) {
+		return this.config.includeNpm || path.indexOf('node_modules') < 0;
 	}
 
 	/**

package.json

@@ -34,15 +34,15 @@
     "lint": "eslint bin/cli.js lib test/*.js",
     "debug": "node bin/cli.js --debug bin lib",
     "generate": "npm run generate:small && npm run generate:madge",
-    "generate:small": "bin/cli.js --image /tmp/simple.svg test/commonjs/circular/a.js",
+    "generate:small": "bin/cli.js --image /tmp/simple.svg test/cjs/circular/a.js",
     "generate:madge": "bin/cli.js --image /tmp/madge.svg bin lib"
   },
   "dependencies": {
     "chalk": "^1.1.3",
     "commander": "^2.9.0",
     "commondir": "^1.0.1",
     "debug": "^2.2.0",
-    "dependency-tree": "^5.5.1",
+    "dependency-tree": "^5.5.3",
     "graphviz": "^0.0.8",
     "mz": "^2.4.0",
     "rc": "^1.1.6",

test/api.js

@@ -16,7 +16,7 @@ describe('API', () => {
 	});
 
 	it('returns a Promise', () => {
-		madge(__dirname + '/commonjs/a.js').should.be.Promise(); // eslint-disable-line new-cap
+		madge(__dirname + '/cjs/a.js').should.be.Promise(); // eslint-disable-line new-cap
 	});
 
 	it('throws error if file or directory does not exists', (done) => {
@@ -27,7 +27,7 @@ describe('API', () => {
 	});
 
 	it('takes single file as path', (done) => {
-		madge(__dirname + '/commonjs/a.js').then((res) => {
+		madge(__dirname + '/cjs/a.js').then((res) => {
 			res.obj().should.eql({
 				'a': ['b', 'c'],
 				'b': ['c'],
@@ -38,7 +38,7 @@ describe('API', () => {
 	});
 
 	it('takes an array of files as path and combines the result', (done) => {
-		madge([__dirname + '/commonjs/a.js', __dirname + '/commonjs/normal/d.js']).then((res) => {
+		madge([__dirname + '/cjs/a.js', __dirname + '/cjs/normal/d.js']).then((res) => {
 			res.obj().should.eql({
 				'a': ['b', 'c'],
 				'b': ['c'],
@@ -50,7 +50,7 @@ describe('API', () => {
 	});
 
 	it('take a single directory as path and find files in it', (done) => {
-		madge(__dirname + '/commonjs/normal').then((res) => {
+		madge(__dirname + '/cjs/normal').then((res) => {
 			res.obj().should.eql({
 				'a': ['sub/b'],
 				'd': [],
@@ -62,7 +62,7 @@ describe('API', () => {
 	});
 
 	it('takes an array of directories as path and compute the basedir correctly', (done) => {
-		madge([__dirname + '/commonjs/multibase/1', __dirname + '/commonjs/multibase/2']).then((res) => {
+		madge([__dirname + '/cjs/multibase/1', __dirname + '/cjs/multibase/2']).then((res) => {
 			res.obj().should.eql({
 				'1/a': [],
 				'2/b': []
@@ -89,7 +89,7 @@ describe('API', () => {
 	});
 
 	it('can exclude modules using RegExp', (done) => {
-		madge(__dirname + '/commonjs/a.js', {
+		madge(__dirname + '/cjs/a.js', {
 			excludeRegExp: ['^b$']
 		}).then((res) => {
 			res.obj().should.eql({
@@ -102,7 +102,7 @@ describe('API', () => {
 
 	describe('obj()', () => {
 		it('returns dependency object', (done) => {
-			madge(__dirname + '/commonjs/a.js').then((res) => {
+			madge(__dirname + '/cjs/a.js').then((res) => {
 				res.obj().should.eql({
 					a: ['b', 'c'],
 					b: ['c'],
@@ -115,7 +115,7 @@ describe('API', () => {
 
 	describe('dot()', () => {
 		it('returns a promise resolved with graphviz DOT output', (done) => {
-			madge(__dirname + '/commonjs/b.js')
+			madge(__dirname + '/cjs/b.js')
 				.then((res) => res.dot())
 				.then((output) => {
 					output.should.eql('digraph G {\n  "b";\n  "c";\n  "b" -> "c";\n}\n');
@@ -127,7 +127,7 @@ describe('API', () => {
 
 	describe('depends()', () => {
 		it('returns modules that depends on another', (done) => {
-			madge(__dirname + '/commonjs/a.js').then((res) => {
+			madge(__dirname + '/cjs/a.js').then((res) => {
 				res.depends('c').should.eql(['a', 'b']);
 				done();
 			}).catch(done);
@@ -146,7 +146,7 @@ describe('API', () => {
 		});
 
 		it('rejects if a filename is not supplied', (done) => {
-			madge(__dirname + '/commonjs/a.js')
+			madge(__dirname + '/cjs/a.js')
 				.then((res) => res.image())
 				.catch((err) => {
 					err.message.should.eql('imagePath not provided');
@@ -155,7 +155,7 @@ describe('API', () => {
 		});
 
 		it('rejects on unsupported image format', (done) => {
-			madge(__dirname + '/commonjs/a.js')
+			madge(__dirname + '/cjs/a.js')
 				.then((res) => res.image('image.zyx'))
 				.catch((err) => {
 					err.message.should.match(/Format: "zyx" not recognized/);
@@ -164,7 +164,7 @@ describe('API', () => {
 		});
 
 		it('rejects if graphviz is not installed', (done) => {
-			madge(__dirname + '/commonjs/a.js', {graphVizPath: '/invalid/path'})
+			madge(__dirname + '/cjs/a.js', {graphVizPath: '/invalid/path'})
 				.then((res) => res.image('image.png'))
 				.catch((err) => {
 					err.message.should.match(/Could not execute .*gvpr \-V/);
@@ -173,7 +173,7 @@ describe('API', () => {
 		});
 
 		it('writes image to file', (done) => {
-			madge(__dirname + '/commonjs/a.js')
+			madge(__dirname + '/cjs/a.js')
 				.then((res) => res.image(imagePath))
 				.then((writtenImagePath) => {
 					writtenImagePath.should.eql(imagePath);

test/commonjs.js → test/cjs.js

@@ -5,7 +5,7 @@ const madge = require('../lib/api');
 require('should');
 
 describe('CommonJS', () => {
-	const dir = __dirname + '/commonjs';
+	const dir = __dirname + '/cjs';
 
 	it('finds recursive dependencies', (done) => {
 		madge(dir + '/normal/a.js').then((res) => {

test/flow.js

@@ -18,7 +18,7 @@ describe('Flow', () => {
 	});
 
 	it('extracts CommonsJS module dependencies', (done) => {
-		madge(dir + '/commonjs/calc.js').then((res) => {
+		madge(dir + '/cjs/calc.js').then((res) => {
 			res.obj().should.eql({
 				'math': [],
 				'calc': ['math']

test/jsx/basic.jsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import other from './other';
+import other from './other.jsx';
 
 export default React.createClass({
   render: function() {