Merge pull request #859 from mjbvz/string-intern-memory-improvements

Analysis String interning cleanup

Author: mjbvz

Nodejs/Product/Analysis/Analysis.csproj

@@ -191,7 +191,7 @@
     <Compile Include="Helpers.cs" />
     <Compile Include="JavaScript\activationobject.cs" />
     <Compile Include="JavaScript\arrayliteral.cs" />
-    <Compile Include="JavaScript\CacheDict.cs" />
+    <Compile Include="JavaScript\StringInternPool.cs" />
     <Compile Include="JavaScript\EncodedSpan.cs" />
     <Compile Include="JavaScript\Node.cs" />
     <Compile Include="JavaScript\binaryop.cs" />

Nodejs/Product/Analysis/JavaScript/CacheDict.cs

@@ -0,0 +1,72 @@
+//*********************************************************//
+//    Copyright (c) Microsoft. All rights reserved.
+//    
+//    Apache 2.0 License
+//    
+//    You may obtain a copy of the License at
+//    http://www.apache.org/licenses/LICENSE-2.0
+//    
+//    Unless required by applicable law or agreed to in writing, software 
+//    distributed under the License is distributed on an "AS IS" BASIS, 
+//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
+//    implied. See the License for the specific language governing 
+//    permissions and limitations under the License.
+//
+//*********************************************************//
+
+using System.Collections.Generic;
+using System.Diagnostics;
+
+namespace Microsoft.NodejsTools.Parsing {
+    /// <summary>
+    /// Pool that caches strings for memory reuse.
+    /// 
+    /// Used over `String.Intern` since that cannot release held strings.
+    /// 
+    /// This class is not thread safe.
+    /// </summary>
+    internal class StringInternPool {
+        private readonly Dictionary<string, LinkedListNode<string>> _dict = new Dictionary<string, LinkedListNode<string>>();
+        private readonly LinkedList<string> _list = new LinkedList<string>();
+        private readonly int _maxSize;
+
+        /// <summary>
+        /// Creates a new string pool.
+        /// </summary>
+        /// <param name="maxSize">The maximum number of elements to store.</param>
+        internal StringInternPool(int maxSize) {
+            _maxSize = maxSize;
+        }
+
+        /// <summary>
+        /// Interns a string.
+        /// Returns the reused string if the string is already in the cache, otherwise adds the string to the cache.
+        /// </summary>
+        internal  string Intern(string str) {
+            if (string.IsNullOrEmpty(str)) {
+                return string.Empty;
+            }
+
+            LinkedListNode<string> existingNode;
+            if (_dict.TryGetValue(str, out existingNode) && existingNode != null) {
+                // Move node to head of list
+                _list.Remove(existingNode);
+                _list.AddFirst(existingNode);
+                return existingNode.Value;
+            }
+
+            // Trim the size of the cache.
+            while (_list.Count >= _maxSize) {
+                var last = _list.Last;
+                _list.RemoveLast();
+                bool res = _dict.Remove(last.Value);
+                Debug.Assert(res);
+            }
+            // Add new value to head of list
+            var newNode = new LinkedListNode<string>(str);
+            _list.AddFirst(newNode);
+            _dict[newNode.Value] = newNode;
+            return newNode.Value;
+        }
+    }
+}

Nodejs/Product/Analysis/JavaScript/StringInternPool.cs

@@ -50,7 +50,7 @@ internal sealed class JSScanner
         private readonly ErrorSink _errorSink;
         private readonly CodeSettings _settings;
 
-        private static CacheDict<string, string> _internTable = new CacheDict<string, string>(4096);
+        private static StringInternPool _internTable = new StringInternPool(4096);
 
         #endregion
 
@@ -80,10 +80,8 @@ public string StringLiteralValue {
         // Whether the current multiline comment is a doclet.
         public bool IsDoclet { get; private set; }
 
-        internal string Identifier
-        {
-            get
-            {
+        internal string Identifier {
+            get {
                 return _identifier.Length > 0
                     ? InternString(_identifier.ToString()) :
                     CurrentTokenString();
@@ -92,12 +90,7 @@ internal string Identifier
 
         private static string InternString(string value) {
             lock (_internTable) {
-                string res;
-                if (_internTable.TryGetValue(value, out res)) {
-                    return res;
-                }
-                _internTable.Add(value, value);
-                return value;
+                return _internTable.Intern(value);
             }
         }