From 6c8114cf75cc4ad92116d5fe6bdcf43fd9a13d6e Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 9 Sep 2016 14:10:54 -0700 Subject: [PATCH 1/2] Limit Depth of Npm Modules This is a resbumission of #944 **Bug** Now that our Node analyzer cannot be used in the main editor, there no real reason why we need to build a complete tree of npm installs. As #944 documented, this is a major source of memory and cpu usage at boot **Fix** Limit depth of explored npm tree to a single level. This brings some very nice perf gains for projects that have complex depdency tress. --- Nodejs/Product/Npm/SPI/NodeModules.cs | 8 ++++++-- Nodejs/Product/Npm/SPI/RootPackage.cs | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Nodejs/Product/Npm/SPI/NodeModules.cs b/Nodejs/Product/Npm/SPI/NodeModules.cs index ec36c0644..3fd3af7ca 100644 --- a/Nodejs/Product/Npm/SPI/NodeModules.cs +++ b/Nodejs/Product/Npm/SPI/NodeModules.cs @@ -22,10 +22,14 @@ namespace Microsoft.NodejsTools.Npm.SPI { internal class NodeModules : AbstractNodeModules { - private Dictionary _allModules; + private readonly Dictionary _allModules; private static readonly string[] _ignoredDirectories = { @"\.bin", @"\.staging" }; - public NodeModules(IRootPackage parent, bool showMissingDevOptionalSubPackages, Dictionary allModulesToDepth = null, int depth = 0) { + public NodeModules(IRootPackage parent, bool showMissingDevOptionalSubPackages, Dictionary allModulesToDepth = null, int depth = 0, int maxDepth = 1) { + if (depth >= maxDepth) { + return; + } + var modulesBase = Path.Combine(parent.Path, NodejsConstants.NodeModulesFolder); _allModules = allModulesToDepth ?? new Dictionary(); diff --git a/Nodejs/Product/Npm/SPI/RootPackage.cs b/Nodejs/Product/Npm/SPI/RootPackage.cs index a295ee9f4..937c68055 100644 --- a/Nodejs/Product/Npm/SPI/RootPackage.cs +++ b/Nodejs/Product/Npm/SPI/RootPackage.cs @@ -24,7 +24,8 @@ public RootPackage( string fullPathToRootDirectory, bool showMissingDevOptionalSubPackages, Dictionary allModules = null, - int depth = 0) { + int depth = 0, + int maxDepth = 1) { Path = fullPathToRootDirectory; var packageJsonFile = System.IO.Path.Combine(fullPathToRootDirectory, "package.json"); try { @@ -44,7 +45,7 @@ public RootPackage( } try { - Modules = new NodeModules(this, showMissingDevOptionalSubPackages, allModules, depth); + Modules = new NodeModules(this, showMissingDevOptionalSubPackages, allModules, depth, maxDepth); } catch (PathTooLongException) { // otherwise we fail to create it completely... } From 38974c504720833ec05bf02e845304d031961a0c Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 9 Sep 2016 15:21:24 -0700 Subject: [PATCH 2/2] Fix unit test for recurisve read --- Nodejs/Product/Npm/RootPackageFactory.cs | 8 ++++++-- Nodejs/Product/Npm/SPI/NodeModules.cs | 14 +++++++------- Nodejs/Product/Npm/SPI/Package.cs | 5 +++-- Nodejs/Tests/NpmTests/ModuleHierarchyTests.cs | 2 +- 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Nodejs/Product/Npm/RootPackageFactory.cs b/Nodejs/Product/Npm/RootPackageFactory.cs index a7b9dd06a..a3c4d24f7 100644 --- a/Nodejs/Product/Npm/RootPackageFactory.cs +++ b/Nodejs/Product/Npm/RootPackageFactory.cs @@ -20,10 +20,14 @@ namespace Microsoft.NodejsTools.Npm { public static class RootPackageFactory { public static IRootPackage Create( string fullPathToRootDirectory, - bool showMissingDevOptionalSubPackages = false) { + bool showMissingDevOptionalSubPackages = false, + int maxDepth = 1) { return new RootPackage( fullPathToRootDirectory, - showMissingDevOptionalSubPackages); + showMissingDevOptionalSubPackages, + null, + 0, + maxDepth); } } } \ No newline at end of file diff --git a/Nodejs/Product/Npm/SPI/NodeModules.cs b/Nodejs/Product/Npm/SPI/NodeModules.cs index 3fd3af7ca..f0883be75 100644 --- a/Nodejs/Product/Npm/SPI/NodeModules.cs +++ b/Nodejs/Product/Npm/SPI/NodeModules.cs @@ -48,13 +48,13 @@ public NodeModules(IRootPackage parent, bool showMissingDevOptionalSubPackages, // _requiredBy dependencies that begin with hash characters represent top-level dependencies foreach (var requiredBy in packageJson.RequiredBy) { if (requiredBy.StartsWith("#") || requiredBy == "/") { - AddTopLevelModule(parent, showMissingDevOptionalSubPackages, moduleDir, depth); + AddTopLevelModule(parent, showMissingDevOptionalSubPackages, moduleDir, depth, maxDepth); break; } } } else { // This dependency is a top-level dependency not added by npm v3 - AddTopLevelModule(parent, showMissingDevOptionalSubPackages, moduleDir, depth); + AddTopLevelModule(parent, showMissingDevOptionalSubPackages, moduleDir, depth, maxDepth); } } } @@ -71,7 +71,7 @@ public NodeModules(IRootPackage parent, bool showMissingDevOptionalSubPackages, // try to find folder by recursing up tree do { moduleDir = Path.Combine(moduleDir, dependency.Name); - if (AddModuleIfNotExists(parent, moduleDir, showMissingDevOptionalSubPackages, depth, dependency)) { + if (AddModuleIfNotExists(parent, moduleDir, showMissingDevOptionalSubPackages, depth, maxDepth, dependency)) { break; } @@ -84,12 +84,12 @@ public NodeModules(IRootPackage parent, bool showMissingDevOptionalSubPackages, _packagesSorted.Sort(new PackageComparer()); } - private void AddTopLevelModule(IRootPackage parent, bool showMissingDevOptionalSubPackages, string moduleDir, int depth) { + private void AddTopLevelModule(IRootPackage parent, bool showMissingDevOptionalSubPackages, string moduleDir, int depth, int maxDepth) { Debug.Assert(depth == 0, "Depth should be 0 when adding a top level dependency"); - AddModuleIfNotExists(parent, moduleDir, showMissingDevOptionalSubPackages, depth); + AddModuleIfNotExists(parent, moduleDir, showMissingDevOptionalSubPackages, depth, maxDepth); } - private bool AddModuleIfNotExists(IRootPackage parent, string moduleDir, bool showMissingDevOptionalSubPackages, int depth, IDependency dependency = null) { + private bool AddModuleIfNotExists(IRootPackage parent, string moduleDir, bool showMissingDevOptionalSubPackages, int depth, int maxDepth, IDependency dependency = null) { depth++; ModuleInfo moduleInfo; @@ -124,7 +124,7 @@ private bool AddModuleIfNotExists(IRootPackage parent, string moduleDir, bool sh moduleInfo.RequiredBy.Add(parent.Path); - var pkg = new Package(parent, moduleDir, showMissingDevOptionalSubPackages, _allModules, depth); + var pkg = new Package(parent, moduleDir, showMissingDevOptionalSubPackages, _allModules, depth, maxDepth); if (dependency != null) { pkg.RequestedVersionRange = dependency.VersionRangeText; } diff --git a/Nodejs/Product/Npm/SPI/Package.cs b/Nodejs/Product/Npm/SPI/Package.cs index d46c96148..3de992a96 100644 --- a/Nodejs/Product/Npm/SPI/Package.cs +++ b/Nodejs/Product/Npm/SPI/Package.cs @@ -28,8 +28,9 @@ public Package( string fullPathToRootDirectory, bool showMissingDevOptionalSubPackages, Dictionary allModules = null, - int depth = 0) - : base(fullPathToRootDirectory, showMissingDevOptionalSubPackages, allModules, depth) { + int depth = 0, + int maxDepth = 1) + : base(fullPathToRootDirectory, showMissingDevOptionalSubPackages, allModules, depth, maxDepth) { _parent = parent; } diff --git a/Nodejs/Tests/NpmTests/ModuleHierarchyTests.cs b/Nodejs/Tests/NpmTests/ModuleHierarchyTests.cs index 74aec7439..fbba5e173 100644 --- a/Nodejs/Tests/NpmTests/ModuleHierarchyTests.cs +++ b/Nodejs/Tests/NpmTests/ModuleHierarchyTests.cs @@ -156,7 +156,7 @@ public void ReadRootDependencyRecursive() { var rootDir = FilesystemPackageJsonTestHelpers.CreateRootPackage(manager, PkgSingleRecursiveDependency); RunNpmInstall(rootDir); - var pkg = RootPackageFactory.Create(rootDir); + var pkg = RootPackageFactory.Create(rootDir, false, int.MaxValue); var json = pkg.PackageJson; var dependencies = json.AllDependencies;