Improve ArrayList and NodeList

* better indexer with less IL * encapsulate EnsureCapacity * remove TrimExcess
sebastienros · Aug 10, 2023 · e82b68d · e82b68d
1 parent 7d26304
commit e82b68d
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 75 deletions.
diff --git a/samples/Esprima.Benchmark/NodeListEnumerationBenchmark.cs b/samples/Esprima.Benchmark/NodeListEnumerationBenchmark.cs
@@ -3,10 +3,8 @@
 
 namespace Esprima.Benchmark;
 
-[MemoryDiagnoser]
 [RankColumn]
-[CsvExporter]
-[MarkdownExporterAttribute.GitHub]
+[MemoryDiagnoser]
 public class NodeListEnumerationBenchmark
 {
     private const string FileName = "bundle";
@@ -30,7 +28,7 @@ public void Setup()
     }
 
     [Benchmark]
-    public object For_DirectIndexing()
+    public int For_DirectIndexing()
     {
         var nodeList = _nodeLists[NodeListIndex];
 
@@ -43,7 +41,7 @@ public object For_DirectIndexing()
     }
 
     [Benchmark]
-    public object For_SpanIndexing()
+    public int For_SpanIndexing()
     {
         var nodeList = _nodeLists[NodeListIndex].AsSpan();
 
@@ -56,7 +54,7 @@ public object For_SpanIndexing()
     }
 
     [Benchmark]
-    public object ForEach_Span()
+    public int ForEach_Span()
     {
         var nodeList = _nodeLists[NodeListIndex].AsSpan();
 

diff --git a/samples/Esprima.Benchmark/Program.cs b/samples/Esprima.Benchmark/Program.cs
@@ -1,12 +1,4 @@
 using System.Reflection;
 using BenchmarkDotNet.Running;
 
-namespace Esprima.Benchmark;
-
-public class Program
-{
-    public static void Main(string[] args)
-    {
-        BenchmarkSwitcher.FromAssembly(typeof(Program).GetTypeInfo().Assembly).Run(args);
-    }
-}
+BenchmarkSwitcher.FromAssembly(typeof(Program).GetTypeInfo().Assembly).Run(args);
diff --git a/src/Esprima/ArrayList.cs b/src/Esprima/ArrayList.cs
@@ -80,7 +80,7 @@ public ArrayList(int initialCapacity)
     {
         if (initialCapacity < 0)
         {
-            ThrowArgumentOutOfRangeException<T>(nameof(initialCapacity), initialCapacity, null);
+            ThrowInvalidInitialCapacity();
         }
 
         _items = initialCapacity > 0 ? new T[initialCapacity] : null;
@@ -90,6 +90,12 @@ public ArrayList(int initialCapacity)
         _localVersion = 0;
         _sharedVersion = null;
 #endif
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static void ThrowInvalidInitialCapacity()
+        {
+            ThrowArgumentException("Invalid initial capacity", nameof(initialCapacity));
+        }
     }
 
     /// <remarks>
@@ -108,6 +114,7 @@ internal ArrayList(T[] items)
 
     public int Capacity
     {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         get
         {
             AssertUnchanged();
@@ -117,28 +124,13 @@ public int Capacity
         {
             AssertUnchanged();
 
-            if (value < _count)
-            {
-                ThrowArgumentOutOfRangeException<T>(nameof(value), value, null);
-            }
-            else if (value == (_items?.Length ?? 0))
-            {
-                return;
-            }
-            else if (value > 0)
+            if (value < (_items?.Length ?? 0))
             {
-                T[] array = new T[value];
-                if (_count > 0)
-                {
-                    Array.Copy(_items, 0, array, 0, _count);
-                }
-                _items = array;
-            }
-            else
-            {
-                _items = null;
+                ThrowArgumentOutOfRangeException(nameof(value), value, null);
             }
 
+            EnsureCapacity(value);
+
             OnChanged();
         }
     }
@@ -166,7 +158,7 @@ public T this[int index]
                 return _items![index];
             }
 
-            return ThrowArgumentOutOfRangeException<T>(nameof(index), index, null);
+            return ThrowIndexOutOfRangeException<T>();
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
@@ -181,7 +173,7 @@ public T this[int index]
                 return;
             }
 
-            ThrowArgumentOutOfRangeException<T>(nameof(index), index, null);
+            ThrowIndexOutOfRangeException<T>();
         }
     }
 
@@ -221,10 +213,7 @@ public void AddRange(ReadOnlySpan<T> items)
         var oldCount = _count;
         var newCount = oldCount + itemCount;
 
-        if (Capacity < newCount)
-        {
-            Array.Resize(ref _items, Math.Max(newCount, MinAllocatedCount));
-        }
+        EnsureCapacity(newCount);
 
         Debug.Assert(_items is not null);
         items.CopyTo(_items.AsSpan(oldCount, itemCount));
@@ -242,12 +231,7 @@ public void Add(T item)
     {
         AssertUnchanged();
 
-        var capacity = Capacity;
-
-        if (_count == capacity)
-        {
-            Array.Resize(ref _items, Math.Max(capacity * 2, MinAllocatedCount));
-        }
+        EnsureCapacity(_count + 1);
 
         Debug.Assert(_items is not null);
         _items![_count] = item;
@@ -275,15 +259,10 @@ public void Insert(int index, T item)
 
         if ((uint) index > (uint) _count)
         {
-            ThrowArgumentOutOfRangeException<T>(nameof(index), index, null);
+            ThrowIndexOutOfRangeException<T>();
         }
 
-        var capacity = Capacity;
-
-        if (_count == capacity)
-        {
-            Array.Resize(ref _items, Math.Max(capacity * 2, MinAllocatedCount));
-        }
+        EnsureCapacity(_count + 1);
 
         Debug.Assert(_items is not null);
         Array.Copy(_items, index, _items, index + 1, Count - index);
@@ -299,7 +278,7 @@ public void RemoveAt(int index)
 
         if ((uint) index >= (uint) _count)
         {
-            ThrowArgumentOutOfRangeException<T>(nameof(index), index, null);
+            ThrowIndexOutOfRangeException<T>();
         }
 
         _count--;
@@ -348,16 +327,6 @@ public void Yield(out T[]? items, out int count)
         this = default;
     }
 
-    public void TrimExcess(int threshold = MinAllocatedCount)
-    {
-        AssertUnchanged();
-
-        if (Capacity - _count > threshold)
-        {
-            Capacity = _count;
-        }
-    }
-
     /// <remarks>
     /// Items should not be added or removed from the <see cref="ArrayList{T}"/> while the returned <see cref="Span{T}"/> is in use!
     /// </remarks>
@@ -398,6 +367,18 @@ IEnumerator IEnumerable.GetEnumerator()
         return GetEnumerator();
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private void EnsureCapacity(int capacity)
+    {
+        var temp = _items;
+        if (temp is null || temp.Length < capacity)
+        {
+            var minAllocatedCount = temp is null || temp.Length < MinAllocatedCount ? MinAllocatedCount : temp.Length * 2;
+            var newSize = Math.Max(capacity, minAllocatedCount);
+            Array.Resize(ref _items, newSize);
+        }
+    }
+
     /// <remarks>
     /// This implementation does not detect changes to the list
     /// during iteration and therefore the behaviour is undefined

diff --git a/src/Esprima/Ast/NodeList.cs b/src/Esprima/Ast/NodeList.cs
@@ -14,15 +14,20 @@ internal NodeList(ICollection<T> collection)
     {
         if (collection is null)
         {
-            throw new ArgumentNullException(nameof(collection));
+            ThrowArgumentNullException();
         }
 
-        _count = collection.Count;
+        _count = collection!.Count;
         if (_count > 0)
         {
             _items = new T[_count];
             collection.CopyTo(_items, 0);
         }
+
+        static void ThrowArgumentNullException()
+        {
+            ThrowArgumentNullException<T>(nameof(collection));
+        }
     }
 
     /// <remarks>
@@ -53,7 +58,7 @@ public T this[int index]
                 return _items![index];
             }
 
-            return ThrowArgumentOutOfRangeException<T>(nameof(index), index, null);
+            return ThrowIndexOutOfRangeException<T>();
         }
     }
 

diff --git a/src/Esprima/EsprimaExceptionHelper.cs b/src/Esprima/EsprimaExceptionHelper.cs
@@ -8,17 +8,29 @@ namespace Esprima;
 internal static class EsprimaExceptionHelper
 {
     [DoesNotReturn]
-    public static T ThrowArgumentNullException<T>(string message)
+    public static T ThrowArgumentNullException<T>(string paramName)
     {
-        throw new ArgumentNullException(message);
+        throw new ArgumentNullException(paramName);
     }
 
     [DoesNotReturn]
-    public static T ThrowArgumentOutOfRangeException<T>(string paramName, object actualValue, string? message = null)
+    public static void ThrowArgumentOutOfRangeException<T>(string paramName, T actualValue, string? message = null)
     {
         throw new ArgumentOutOfRangeException(paramName, actualValue, message);
     }
 
+    [DoesNotReturn]
+    public static void ThrowArgumentException(string message, string paramName)
+    {
+        throw new ArgumentException(message, paramName);
+    }
+
+    [DoesNotReturn]
+    public static T ThrowIndexOutOfRangeException<T>()
+    {
+        throw new ArgumentOutOfRangeException("index");
+    }
+
     [DoesNotReturn]
     public static T ThrowFormatException<T>(string message)
     {

diff --git a/src/Esprima/JavascriptParser.cs b/src/Esprima/JavascriptParser.cs
@@ -50,7 +50,7 @@ public void ReleaseLargeBuffers()
             Decorators.Clear();
             if (Decorators.Capacity > 64)
             {
-                Decorators.Capacity = 64;
+                Decorators = new ArrayList<Decorator>(64);
             }
 
             if (LabelSet.Count > 64)

diff --git a/src/Esprima/Scanner.RegExpParser.cs b/src/Esprima/Scanner.RegExpParser.cs
@@ -185,7 +185,6 @@ public RegExpParseResult Parse()
             }
 
             Debug.Assert(conversionError is null);
-            capturingGroups.TrimExcess();
 
             var options = FlagsToOptions(_flags, compiled: _scanner._regExpParseMode == RegExpParseMode.AdaptToCompiled);
             var matchTimeout = _scanner._regexTimeout;

diff --git a/src/Esprima/Scanner.cs b/src/Esprima/Scanner.cs
@@ -144,7 +144,7 @@ internal void ReleaseLargeBuffers()
         _curlyStack.Clear();
         if (_curlyStack.Capacity > 16)
         {
-            _curlyStack.Capacity = 16;
+            _curlyStack = new ArrayList<string>(16);
         }
 
         _sb.Clear();
@@ -517,7 +517,6 @@ internal ArrayList<Comment> ScanCommentsInternal()
     public ReadOnlySpan<Comment> ScanComments()
     {
         var comments = ScanCommentsInternal();
-        comments.TrimExcess();
         return comments.AsSpan();
     }