Browse Source

Reduce func allocations (#3919)

* Replace Region.Contains LINQ lambdas with foreach loop

Removes the lambda func allocations caused by captured outer variables.

* Replace LineCanvas.Has LINQ lambda with foreach loop

* Fix LineCanvas.GetMap intersects array nullability

It should be enough to add null-forgiving operator somewhere in the LINQ query to make the final result non-null. No need to shove the nullability further down the line to complicate things. :)

* Replace LineCanvas.All LINQ lambda with foreach loop

* Replace Region.Intersect LINQ lambdas and list allocation with foreach loop and rented array

* Use stackalloc buffer in Region.Intersect when max 8 rectangles

* Fix LineCanvas.GetCellMap intersects array nullability

* Remove leftover LineCanvas.GetRuneForIntersects null-conditional operators

* Remove leftover IntersectionRuneResolver.GetRuneForIntersects null-conditional operators

* PosAlign.CalculateMinDimension: calculate sum during loop

No need to first put the dimensions in a list and then sum the list when you can just add to sum while looping through dimensions.

* PosAlign.CalculateMinDimension: Remove intermediate list and related filter func allocation

* TextFormatter.GetRuneWidth: Remove intermediate list and related sum func allocation

* ReadOnlySpan refactor preparation for GetCellMap rewrite

* LineCanvas.GetCellMap: Reuse intersection list outside nested loops

GetCellMap would not benefit much from array pool because IntersectionDefinition is not struct. This  change eliminates majority of the rest of Func<,> allocations. As a bonus IntersectionDefinition[] allocations dropped nicely.

* Refactor local method UseRounded

* Wrap too long list of method parameters

* Region: Consistent location for #nullable enable

---------

Co-authored-by: Tig <[email protected]>
Tonttu 5 months ago
parent
commit
35522cc517

+ 128 - 73
Terminal.Gui/Drawing/LineCanvas/LineCanvas.cs

@@ -1,4 +1,7 @@
 #nullable enable
+using System.Buffers;
+using System.Runtime.InteropServices;
+
 namespace Terminal.Gui;
 
 /// <summary>Facilitates box drawing and line intersection detection and rendering. Does not support diagonal lines.</summary>
@@ -161,18 +164,25 @@ public class LineCanvas : IDisposable
     {
         Dictionary<Point, Cell?> map = new ();
 
+        List<IntersectionDefinition> intersectionsBufferList = [];
+
         // walk through each pixel of the bitmap
         for (int y = Bounds.Y; y < Bounds.Y + Bounds.Height; y++)
         {
             for (int x = Bounds.X; x < Bounds.X + Bounds.Width; x++)
             {
-                IntersectionDefinition? [] intersects = _lines
-                                                        .Select (l => l.Intersects (x, y))
-                                                        .Where (i => i is { })
-                                                        .ToArray ();
-
+                intersectionsBufferList.Clear ();
+                foreach (var line in _lines)
+                {
+                    if (line.Intersects (x, y) is IntersectionDefinition intersect)
+                    {
+                        intersectionsBufferList.Add (intersect);
+                    }
+                }
+                // Safe as long as the list is not modified while the span is in use.
+                ReadOnlySpan<IntersectionDefinition> intersects = CollectionsMarshal.AsSpan(intersectionsBufferList);
                 Cell? cell = GetCellForIntersects (Application.Driver, intersects);
-
+                // TODO: Can we skip the whole nested looping if _exclusionRegion is null?
                 if (cell is { } && _exclusionRegion?.Contains (x, y) is null or false)
                 {
                     map.Add (new (x, y), cell);
@@ -207,10 +217,11 @@ public class LineCanvas : IDisposable
         {
             for (int x = inArea.X; x < inArea.X + inArea.Width; x++)
             {
-                IntersectionDefinition? [] intersects = _lines
-                                                        .Select (l => l.Intersects (x, y))
-                                                        .Where (i => i is { })
-                                                        .ToArray ();
+                IntersectionDefinition [] intersects = _lines
+                    // ! nulls are filtered out by the next Where filter
+                    .Select (l => l.Intersects (x, y)!)
+                    .Where (i => i is not null)
+                    .ToArray ();
 
                 Rune? rune = GetRuneForIntersects (Application.Driver, intersects);
 
@@ -315,9 +326,16 @@ public class LineCanvas : IDisposable
         return sb.ToString ();
     }
 
-    private static bool All (IntersectionDefinition? [] intersects, Orientation orientation)
+    private static bool All (ReadOnlySpan<IntersectionDefinition> intersects, Orientation orientation)
     {
-        return intersects.All (i => i!.Line.Orientation == orientation);
+        foreach (var intersect in intersects)
+        {
+            if (intersect.Line.Orientation != orientation)
+            {
+                return false;
+            }
+        }
+        return true;
     }
 
     private void ConfigurationManager_Applied (object? sender, ConfigurationManagerEventArgs e)
@@ -337,9 +355,9 @@ public class LineCanvas : IDisposable
     /// <returns></returns>
     private static bool Exactly (HashSet<IntersectionType> intersects, params IntersectionType [] types) { return intersects.SetEquals (types); }
 
-    private Attribute? GetAttributeForIntersects (IntersectionDefinition? [] intersects)
+    private Attribute? GetAttributeForIntersects (ReadOnlySpan<IntersectionDefinition> intersects)
     {
-        return Fill?.GetAttribute (intersects [0]!.Point) ?? intersects [0]!.Line.Attribute;
+        return Fill?.GetAttribute (intersects [0].Point) ?? intersects [0].Line.Attribute;
     }
 
     private readonly Dictionary<IntersectionRuneType, IntersectionRuneResolver> _runeResolvers = new ()
@@ -384,9 +402,9 @@ public class LineCanvas : IDisposable
         // TODO: Add other resolvers
     };
 
-    private Cell? GetCellForIntersects (IConsoleDriver? driver, IntersectionDefinition? [] intersects)
+    private Cell? GetCellForIntersects (IConsoleDriver? driver, ReadOnlySpan<IntersectionDefinition> intersects)
     {
-        if (!intersects.Any ())
+        if (intersects.IsEmpty)
         {
             return null;
         }
@@ -404,37 +422,28 @@ public class LineCanvas : IDisposable
         return cell;
     }
 
-    private Rune? GetRuneForIntersects (IConsoleDriver? driver, IntersectionDefinition? [] intersects)
+    private Rune? GetRuneForIntersects (IConsoleDriver? driver, ReadOnlySpan<IntersectionDefinition> intersects)
     {
-        if (!intersects.Any ())
+        if (intersects.IsEmpty)
         {
             return null;
         }
 
         IntersectionRuneType runeType = GetRuneTypeForIntersects (intersects);
-
         if (_runeResolvers.TryGetValue (runeType, out IntersectionRuneResolver? resolver))
         {
             return resolver.GetRuneForIntersects (driver, intersects);
         }
 
         // TODO: Remove these once we have all of the below ported to IntersectionRuneResolvers
-        bool useDouble = intersects.Any (i => i?.Line.Style == LineStyle.Double);
-
-        bool useDashed = intersects.Any (
-                                         i => i?.Line.Style == LineStyle.Dashed
-                                              || i?.Line.Style == LineStyle.RoundedDashed
-                                        );
-
-        bool useDotted = intersects.Any (
-                                         i => i?.Line.Style == LineStyle.Dotted
-                                              || i?.Line.Style == LineStyle.RoundedDotted
-                                        );
+        bool useDouble = AnyLineStyles(intersects, [LineStyle.Double]);
+        bool useDashed = AnyLineStyles(intersects, [LineStyle.Dashed, LineStyle.RoundedDashed]);
+        bool useDotted = AnyLineStyles(intersects, [LineStyle.Dotted, LineStyle.RoundedDotted]);
 
         // horiz and vert lines same as Single for Rounded
-        bool useThick = intersects.Any (i => i?.Line.Style == LineStyle.Heavy);
-        bool useThickDashed = intersects.Any (i => i?.Line.Style == LineStyle.HeavyDashed);
-        bool useThickDotted = intersects.Any (i => i?.Line.Style == LineStyle.HeavyDotted);
+        bool useThick = AnyLineStyles(intersects, [LineStyle.Heavy]);
+        bool useThickDashed = AnyLineStyles(intersects, [LineStyle.HeavyDashed]);
+        bool useThickDotted = AnyLineStyles(intersects, [LineStyle.HeavyDotted]);
 
         // TODO: Support ruler
         //var useRuler = intersects.Any (i => i.Line.Style == LineStyle.Ruler && i.Line.Length != 0);
@@ -493,11 +502,31 @@ public class LineCanvas : IDisposable
                            + runeType
                           );
         }
+
+
+        static bool AnyLineStyles (ReadOnlySpan<IntersectionDefinition> intersects, ReadOnlySpan<LineStyle> lineStyles)
+        {
+            foreach (IntersectionDefinition intersect in intersects)
+            {
+                foreach (LineStyle style in lineStyles)
+                {
+                    if (intersect.Line.Style == style)
+                    {
+                        return true;
+                    }
+                }
+            }
+            return false;
+        }
     }
 
-    private IntersectionRuneType GetRuneTypeForIntersects (IntersectionDefinition? [] intersects)
+    private IntersectionRuneType GetRuneTypeForIntersects (ReadOnlySpan<IntersectionDefinition> intersects)
     {
-        HashSet<IntersectionType> set = new (intersects.Select (i => i!.Type));
+        HashSet<IntersectionType> set = new (capacity: intersects.Length);
+        foreach (var intersect in intersects)
+        {
+            set.Add (intersect.Type);
+        }
 
         #region Cross Conditions
 
@@ -683,7 +712,17 @@ public class LineCanvas : IDisposable
     /// <param name="intersects"></param>
     /// <param name="types"></param>
     /// <returns></returns>
-    private bool Has (HashSet<IntersectionType> intersects, params IntersectionType [] types) { return types.All (t => intersects.Contains (t)); }
+    private bool Has (HashSet<IntersectionType> intersects, params IntersectionType [] types)
+    {
+        foreach (var type in types)
+        {
+            if (!intersects.Contains (type))
+            {
+                return false;
+            }
+        }
+        return true;
+    }
 
     private class BottomTeeIntersectionRuneResolver : IntersectionRuneResolver
     {
@@ -727,45 +766,12 @@ public class LineCanvas : IDisposable
         internal Rune _thickV;
         protected IntersectionRuneResolver () { SetGlyphs (); }
 
-        public Rune? GetRuneForIntersects (IConsoleDriver? driver, IntersectionDefinition? [] intersects)
+        public Rune? GetRuneForIntersects (IConsoleDriver? driver, ReadOnlySpan<IntersectionDefinition> intersects)
         {
-            bool useRounded = intersects.Any (
-                                              i => i?.Line.Length != 0
-                                                   && (
-                                                          i?.Line.Style == LineStyle.Rounded
-                                                          || i?.Line.Style
-                                                          == LineStyle.RoundedDashed
-                                                          || i?.Line.Style
-                                                          == LineStyle.RoundedDotted)
-                                             );
-
             // Note that there aren't any glyphs for intersections of double lines with heavy lines
 
-            bool doubleHorizontal = intersects.Any (
-                                                    l => l?.Line.Orientation == Orientation.Horizontal
-                                                         && l.Line.Style == LineStyle.Double
-                                                   );
-
-            bool doubleVertical = intersects.Any (
-                                                  l => l?.Line.Orientation == Orientation.Vertical
-                                                       && l.Line.Style == LineStyle.Double
-                                                 );
-
-            bool thickHorizontal = intersects.Any (
-                                                   l => l?.Line.Orientation == Orientation.Horizontal
-                                                        && (
-                                                               l.Line.Style == LineStyle.Heavy
-                                                               || l.Line.Style == LineStyle.HeavyDashed
-                                                               || l.Line.Style == LineStyle.HeavyDotted)
-                                                  );
-
-            bool thickVertical = intersects.Any (
-                                                 l => l?.Line.Orientation == Orientation.Vertical
-                                                      && (
-                                                             l.Line.Style == LineStyle.Heavy
-                                                             || l.Line.Style == LineStyle.HeavyDashed
-                                                             || l.Line.Style == LineStyle.HeavyDotted)
-                                                );
+            bool doubleHorizontal = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Horizontal, [LineStyle.Double]);
+            bool doubleVertical = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Vertical, [LineStyle.Double]);
 
             if (doubleHorizontal)
             {
@@ -777,6 +783,11 @@ public class LineCanvas : IDisposable
                 return _doubleV;
             }
 
+            bool thickHorizontal = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Horizontal,
+                [LineStyle.Heavy, LineStyle.HeavyDashed, LineStyle.HeavyDotted]);
+            bool thickVertical = AnyWithOrientationAndAnyLineStyle(intersects, Orientation.Vertical,
+                [LineStyle.Heavy, LineStyle.HeavyDashed, LineStyle.HeavyDotted]);
+
             if (thickHorizontal)
             {
                 return thickVertical ? _thickBoth : _thickH;
@@ -787,7 +798,51 @@ public class LineCanvas : IDisposable
                 return _thickV;
             }
 
-            return useRounded ? _round : _normal;
+            return UseRounded (intersects) ? _round : _normal;
+
+            static bool UseRounded (ReadOnlySpan<IntersectionDefinition> intersects)
+            {
+                foreach (var intersect in intersects)
+                {
+                    if (intersect.Line.Length == 0)
+                    {
+                        continue;
+                    }
+
+                    if (intersect.Line.Style is
+                        LineStyle.Rounded or
+                        LineStyle.RoundedDashed or
+                        LineStyle.RoundedDotted)
+                    {
+                        return true;
+                    }
+                }
+                return false;
+            }
+
+            static bool AnyWithOrientationAndAnyLineStyle (
+                ReadOnlySpan<IntersectionDefinition> intersects,
+                Orientation orientation,
+                ReadOnlySpan<LineStyle> lineStyles)
+            {
+                foreach (var i in intersects)
+                {
+                    if (i.Line.Orientation != orientation)
+                    {
+                        continue;
+                    }
+
+                    // Any line style
+                    foreach (var style in lineStyles)
+                    {
+                        if (i.Line.Style == style)
+                        {
+                            return true;
+                        }
+                    }
+                }
+                return false;
+            }
         }
 
         /// <summary>

+ 62 - 4
Terminal.Gui/Drawing/Region.cs

@@ -1,4 +1,8 @@
-/// <summary>
+#nullable enable
+
+using System.Buffers;
+
+/// <summary>
 ///     Represents a region composed of one or more rectangles, providing methods for union, intersection, exclusion, and
 ///     complement operations.
 /// </summary>
@@ -43,7 +47,41 @@ public class Region : IDisposable
     /// <param name="rectangle">The rectangle to intersect with the region.</param>
     public void Intersect (Rectangle rectangle)
     {
-        _rectangles = _rectangles.Select (r => Rectangle.Intersect (r, rectangle)).Where (r => !r.IsEmpty).ToList ();
+        if (_rectangles.Count == 0)
+        {
+            return;
+        }
+        // TODO: In-place swap within the original list. Does order of intersections matter?
+        // Rectangle = 4 * i32 = 16 B
+        // ~128 B stack allocation
+        const int maxStackallocLength = 8;
+        Rectangle []? rentedArray = null;
+        try
+        {
+            Span<Rectangle> rectBuffer = _rectangles.Count <= maxStackallocLength
+                ? stackalloc Rectangle[maxStackallocLength]
+                : (rentedArray = ArrayPool<Rectangle>.Shared.Rent (_rectangles.Count));
+
+            _rectangles.CopyTo (rectBuffer);
+            ReadOnlySpan<Rectangle> rectangles = rectBuffer[.._rectangles.Count];
+            _rectangles.Clear ();
+
+            foreach (var rect in rectangles)
+            {
+                Rectangle intersection = Rectangle.Intersect (rect, rectangle);
+                if (!intersection.IsEmpty)
+                {
+                    _rectangles.Add (intersection);
+                }
+            }
+        }
+        finally
+        {
+            if (rentedArray != null)
+            {
+                ArrayPool<Rectangle>.Shared.Return (rentedArray);
+            }
+        }
     }
 
     /// <summary>
@@ -154,14 +192,34 @@ public class Region : IDisposable
     /// <param name="x">The x-coordinate of the point.</param>
     /// <param name="y">The y-coordinate of the point.</param>
     /// <returns><c>true</c> if the point is contained within the region; otherwise, <c>false</c>.</returns>
-    public bool Contains (int x, int y) { return _rectangles.Any (r => r.Contains (x, y)); }
+    public bool Contains (int x, int y)
+    {
+        foreach (var rect in _rectangles)
+        {
+            if (rect.Contains (x, y))
+            {
+                return true;
+            }
+        }
+        return false;
+    }
 
     /// <summary>
     ///     Determines whether the specified rectangle is contained within the region.
     /// </summary>
     /// <param name="rectangle">The rectangle to check for containment.</param>
     /// <returns><c>true</c> if the rectangle is contained within the region; otherwise, <c>false</c>.</returns>
-    public bool Contains (Rectangle rectangle) { return _rectangles.Any (r => r.Contains (rectangle)); }
+    public bool Contains (Rectangle rectangle)
+    {
+        foreach (var rect in _rectangles)
+        {
+            if (rect.Contains (rectangle))
+            {
+                return true;
+            }
+        }
+        return false;
+    }
 
     /// <summary>
     ///     Returns an array of rectangles that represent the region.

+ 6 - 6
Terminal.Gui/Text/TextFormatter.cs

@@ -1926,12 +1926,12 @@ public class TextFormatter
 
     private static int GetRuneWidth (string str, int tabWidth, TextDirection textDirection = TextDirection.LeftRight_TopBottom)
     {
-        return GetRuneWidth (str.EnumerateRunes ().ToList (), tabWidth, textDirection);
-    }
-
-    private static int GetRuneWidth (List<Rune> runes, int tabWidth, TextDirection textDirection = TextDirection.LeftRight_TopBottom)
-    {
-        return runes.Sum (r => GetRuneWidth (r, tabWidth, textDirection));
+        int runesWidth = 0;
+        foreach (Rune rune in str.EnumerateRunes ())
+        {
+            runesWidth += GetRuneWidth (rune, tabWidth, textDirection);
+        }
+        return runesWidth;
     }
 
     private static int GetRuneWidth (Rune rune, int tabWidth, TextDirection textDirection = TextDirection.LeftRight_TopBottom)

+ 12 - 19
Terminal.Gui/View/Layout/PosAlign.cs

@@ -1,7 +1,6 @@
 #nullable enable
 
 using System.ComponentModel;
-using System.Text.RegularExpressions;
 
 namespace Terminal.Gui;
 
@@ -61,33 +60,27 @@ public record PosAlign : Pos
     /// <returns></returns>
     public static int CalculateMinDimension (int groupId, IList<View> views, Dimension dimension)
     {
-        List<int> dimensionsList = new ();
-
-        // PERF: If this proves a perf issue, consider caching a ref to this list in each item
-        List<View> viewsInGroup = views.Where (v => HasGroupId (v, dimension, groupId)).ToList ();
-
-        if (viewsInGroup.Count == 0)
+        int dimensionsSum = 0;
+        foreach (var view in views)
         {
-            return 0;
-        }
-
-        // PERF: We iterate over viewsInGroup multiple times here.
-
-        // Update the dimensionList with the sizes of the views
-        for (var index = 0; index < viewsInGroup.Count; index++)
-        {
-            View view = viewsInGroup [index];
+            if (!HasGroupId (view, dimension, groupId)) {
+                continue;
+            }
 
-            PosAlign? posAlign = dimension == Dimension.Width ? view.X as PosAlign : view.Y as PosAlign;
+            PosAlign? posAlign = dimension == Dimension.Width
+                ? view.X as PosAlign
+                : view.Y as PosAlign;
 
             if (posAlign is { })
             {
-                dimensionsList.Add (dimension == Dimension.Width ? view.Frame.Width : view.Frame.Height);
+                dimensionsSum += dimension == Dimension.Width
+                    ? view.Frame.Width
+                    : view.Frame.Height;
             }
         }
 
         // Align
-        return dimensionsList.Sum ();
+        return dimensionsSum;
     }
 
     internal static bool HasGroupId (View v, Dimension dimension, int groupId)