Improve Memory Usage by not holding onto multiple copies of GlobalPackages

Nodejs/Product/Nodejs/Project/NodeModulesNode.cs

@@ -16,9 +16,9 @@
 
 using System;
 using System.Collections.Generic;
-using System.IO;
 using System.Linq;
 using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.NodejsTools.Npm;
 using Microsoft.NodejsTools.NpmUI;
 using Microsoft.VisualStudio;
@@ -30,6 +30,33 @@
 
 namespace Microsoft.NodejsTools.Project {
     internal class NodeModulesNode : AbstractNpmNode {
+        #region StaticFields
+        private static INpmGlobalPackageProvider _cachingGlobalPackageProvider = new CacheGlobalNpmPackageProvider();
+
+        /// <summary>
+        /// `INpmGlobalPackageProvider` that caches `IGlobalPackages` objects for reuse.
+        /// </summary>
+        private class CacheGlobalNpmPackageProvider : INpmGlobalPackageProvider {
+            private Dictionary<string, WeakReference<IGlobalPackages>> _packages = new Dictionary<string, WeakReference<IGlobalPackages>>();
+
+            public async Task<IGlobalPackages> GetGlobalPackages(string pathToNpm) {
+                WeakReference<IGlobalPackages> reference;
+                if (_packages.TryGetValue(pathToNpm, out reference)) {
+                    IGlobalPackages current;
+                    if (reference.TryGetTarget(out current) && current != null) {
+                        return current;
+                    }
+                }
+                var delegateProvider = new Npm.SPI.NpmBinGlobalPackageProvider();
+                var globals = await delegateProvider.GetGlobalPackages(pathToNpm);
+                if (globals != null) {
+                    _packages[pathToNpm] = new WeakReference<IGlobalPackages>(globals);
+                }
+                return globals;
+            }
+        }
+        #endregion
+
         #region Constants
 
         /// <summary>
@@ -66,7 +93,7 @@ internal class NodeModulesNode : AbstractNpmNode {
 
         public NodeModulesNode(NodejsProjectNode root)
             : base(root) {
-            _npmController = DefaultNpmController(_projectNode.ProjectHome, new NpmPathProvider(this));
+            _npmController = DefaultNpmController();
             RegisterWithNpmController(_npmController);
 
             _globalModulesNode = new GlobalModulesNode(root, this);
@@ -131,12 +158,13 @@ public string PathToNpm {
             }
         }
 
-        private static INpmController DefaultNpmController(string projectHome, NpmPathProvider pathProvider) {
+        private INpmController DefaultNpmController() {
             return NpmControllerFactory.Create(
-                projectHome,
+                 _projectNode.ProjectHome,
                 NodejsPackage.Instance.NpmOptionsPage.NpmCachePath,
                 false,
-                pathProvider);
+                new NpmPathProvider(this),
+                _cachingGlobalPackageProvider);
         }
 
         private void RegisterWithNpmController(INpmController controller) {

Nodejs/Product/Nodejs/Project/NodejsProjectNode.cs

@@ -1196,6 +1196,5 @@ public override MSBuildResult Build(string config, string target) {
             }
             return base.Build(config, target);
         }
-
     }
 }

Nodejs/Product/Npm/NpmControllerFactory.cs

@@ -14,21 +14,30 @@
 //
 //*********************************************************//
 
+using System.Threading.Tasks;
 using Microsoft.NodejsTools.Npm.SPI;
 
 namespace Microsoft.NodejsTools.Npm {
-    public class NpmControllerFactory {
+    /// <summary>
+    /// Provides global packages for a given npm executable.
+    /// </summary>
+    public interface INpmGlobalPackageProvider {
+        Task<IGlobalPackages> GetGlobalPackages(string pathToNpm);
+    }
+
+    public static class NpmControllerFactory {
         public static INpmController Create(
             string fullPathToRootPackageDirectory,
             string cachePath,
             bool showMissingDevOptionalSubPackages = false,
             INpmPathProvider npmPathProvider = null,
-            bool useFallbackIfNpmNotFound = true) {
+            INpmGlobalPackageProvider globalPackageProvider = null) {
             return new NpmController(
                 fullPathToRootPackageDirectory,
                 cachePath,
                 showMissingDevOptionalSubPackages,
-                npmPathProvider);
+                npmPathProvider,
+                globalPackageProvider);
         }
     }
 }

Nodejs/Product/Npm/SPI/NpmController.cs

@@ -15,20 +15,19 @@
 //*********************************************************//
 
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
 
 namespace Microsoft.NodejsTools.Npm.SPI {
     internal class NpmController : AbstractNpmLogSource, INpmController {
-
         private IPackageCatalog _sRepoCatalog;
         private string _fullPathToRootPackageDirectory;
         private string _cachePath;
         private bool _showMissingDevOptionalSubPackages;
         private INpmPathProvider _npmPathProvider;
+        private INpmGlobalPackageProvider _globalPackageProvider;
         private IRootPackage _rootPackage;
         private IGlobalPackages _globalPackage;
         private readonly object _lock = new object();
@@ -41,19 +40,20 @@ internal class NpmController : AbstractNpmLogSource, INpmController {
 
         private readonly object _fileBitsLock = new object();
 
-
         private bool _isDisposed;
         private bool _isReloadingModules = false;
 
         public NpmController(
             string fullPathToRootPackageDirectory,
             string cachePath,
             bool showMissingDevOptionalSubPackages = false,
-            INpmPathProvider npmPathProvider = null) {
+            INpmPathProvider npmPathProvider = null,
+            INpmGlobalPackageProvider globalPackageProvider = null) {
             _fullPathToRootPackageDirectory = fullPathToRootPackageDirectory;
             _cachePath = cachePath;
             _showMissingDevOptionalSubPackages = showMissingDevOptionalSubPackages;
             _npmPathProvider = npmPathProvider;
+            _globalPackageProvider = globalPackageProvider ?? new NpmBinGlobalPackageProvider();
 
             _localWatcher = CreateModuleDirectoryWatcherIfDirectoryExists(_fullPathToRootPackageDirectory);
             _globalWatcher = CreateModuleDirectoryWatcherIfDirectoryExists(this.ListBaseDirectory);
@@ -127,11 +127,8 @@ public async Task RefreshAsync() {
                             _fullPathToRootPackageDirectory,
                             _showMissingDevOptionalSubPackages);
 
-                var command = new NpmBinCommand(_fullPathToRootPackageDirectory, true, PathToNpm);
+                GlobalPackages = await _globalPackageProvider.GetGlobalPackages(PathToNpm);
 
-                GlobalPackages = (await command.ExecuteAsync())
-                    ? RootPackageFactory.Create(command.BinDirectory)
-                    : null;
             } catch (IOException) {
                 // Can sometimes happen when packages are still installing because the file may still be used by another process
             } finally {
@@ -364,4 +361,16 @@ public void Dispose() {
             }
         }
     }
+
+    /// <summary>
+    /// Standard `INpmGlobalPackageProvider` that uses `npm bin` to get global packages.
+    /// </summary>
+    public class NpmBinGlobalPackageProvider : INpmGlobalPackageProvider {
+        public async Task<IGlobalPackages> GetGlobalPackages(string pathToNpm) {
+            var command = new NpmBinCommand(String.Empty, true, pathToNpm);
+            return (await command.ExecuteAsync())
+                ? RootPackageFactory.Create(command.BinDirectory)
+                : null;
+        }
+    }
 }