fix: Fixes sending packets and sidesteps a major issue with stackallo…

…c and PGO in .NET 8 (#1607) ### Summary - Works around a sneaky edge case bug in the JIT with stackalloc where sometimes the buffer is not zero'd. - Fixes SendDisplayBoatHS - Fixes sending health bars in the `SendEverything()` logic. - Fixes a bug in sizing for some string helper functions. ### Developer Note We are enabled `SkipLocalsInit` - do not rely on `stackalloc` to be zero'd. To zero the buffer, use `span.Clear();` Closes #1606
modernuo · Nov 21, 2023 · 03f850f · 03f850f
1 parent 79ba1ea
commit 03f850f
Show file tree

Hide file tree

Showing 18 changed files with 67 additions and 76 deletions.
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -27,7 +27,6 @@
         <DefineConstants Condition="'$(IsOSX)'=='true' OR '$(IsLinux)'=='true'">UNIX</DefineConstants>
         <DefineConstants Condition="'$(IsX64)'=='true'">CPU_X64</DefineConstants>
         <DefineConstants Condition="'$(IsArm64)'=='true'">CPU_ARM64</DefineConstants>
-        <DefineConstants Condition="'$(SkipLocalsInitAttribute)'=='true'">NO_LOCAL_INIT</DefineConstants>
         <DefineConstants>MUO</DefineConstants>
         <GitVersionBaseDirectory>$(SolutionDir)</GitVersionBaseDirectory>
         <PredefinedCulturesOnly>false</PredefinedCulturesOnly>

diff --git a/Projects/Server/Json/Converters/Point2DConverter.cs b/Projects/Server/Json/Converters/Point2DConverter.cs
@@ -21,9 +21,10 @@ namespace Server.Json;
 
 public class Point2DConverter : JsonConverter<Point2D>
 {
-    private Point2D DeserializeArray(ref Utf8JsonReader reader)
+    private static Point2D DeserializeArray(ref Utf8JsonReader reader)
     {
         Span<int> data = stackalloc int[2];
+        data.Clear();
         var count = 0;
 
         while (true)
@@ -53,9 +54,10 @@ private Point2D DeserializeArray(ref Utf8JsonReader reader)
         return new Point2D(data[0], data[1]);
     }
 
-    private Point2D DeserializeObj(ref Utf8JsonReader reader)
+    private static Point2D DeserializeObj(ref Utf8JsonReader reader)
     {
         Span<int> data = stackalloc int[2];
+        data.Clear();
 
         while (true)
         {

diff --git a/Projects/Server/Json/Converters/Point3DConverter.cs b/Projects/Server/Json/Converters/Point3DConverter.cs
@@ -21,9 +21,11 @@ namespace Server.Json;
 
 public class Point3DConverter : JsonConverter<Point3D>
 {
-    private Point3D DeserializeArray(ref Utf8JsonReader reader)
+    private static Point3D DeserializeArray(ref Utf8JsonReader reader)
     {
         Span<int> data = stackalloc int[3];
+        data.Clear();
+
         var count = 0;
 
         while (true)
@@ -53,9 +55,10 @@ private Point3D DeserializeArray(ref Utf8JsonReader reader)
         return new Point3D(data[0], data[1], data[2]);
     }
 
-    private Point3D DeserializeObj(ref Utf8JsonReader reader)
+    private static Point3D DeserializeObj(ref Utf8JsonReader reader)
     {
         Span<int> data = stackalloc int[3];
+        data.Clear();
 
         while (true)
         {

diff --git a/Projects/Server/Json/Converters/Rectangle3DConverter.cs b/Projects/Server/Json/Converters/Rectangle3DConverter.cs
@@ -21,9 +21,11 @@ namespace Server.Json;
 
 public class Rectangle3DConverter : JsonConverter<Rectangle3D>
 {
-    private Rectangle3D DeserializeArray(ref Utf8JsonReader reader)
+    private static Rectangle3D DeserializeArray(ref Utf8JsonReader reader)
     {
         Span<int> data = stackalloc int[6];
+        data.Clear();
+
         var count = 0;
 
         while (true)
@@ -53,9 +55,10 @@ private Rectangle3D DeserializeArray(ref Utf8JsonReader reader)
         return new Rectangle3D(data[0], data[1], data[2], data[3], data[4], data[5]);
     }
 
-    private Rectangle3D DeserializeObj(ref Utf8JsonReader reader, JsonSerializerOptions options)
+    private static Rectangle3D DeserializeObj(ref Utf8JsonReader reader, JsonSerializerOptions options)
     {
         Span<int> data = stackalloc int[6];
+        data.Clear();
 
         // 0 - xyzwhd, 1 - x1y1z1x2y2z2, 2 - start/end
         var objType = -1;

diff --git a/Projects/Server/Json/Converters/WorldLocationConverter.cs b/Projects/Server/Json/Converters/WorldLocationConverter.cs
@@ -24,9 +24,11 @@ public class WorldLocationConverter : JsonConverter<WorldLocation>
     private static Point3DConverter _point3DConverter;
     private static MapConverter _mapConverter;
 
-    private WorldLocation DeserializeArray(ref Utf8JsonReader reader)
+    private static WorldLocation DeserializeArray(ref Utf8JsonReader reader)
     {
         Span<int> data = stackalloc int[3];
+        data.Clear();
+
         var count = 0;
         var hasMap = false;
         Map map = null;
@@ -77,9 +79,11 @@ private WorldLocation DeserializeArray(ref Utf8JsonReader reader)
         return new WorldLocation(data[0], data[1], data[2], map);
     }
 
-    private WorldLocation DeserializeObj(ref Utf8JsonReader reader, JsonSerializerOptions options)
+    private static WorldLocation DeserializeObj(ref Utf8JsonReader reader, JsonSerializerOptions options)
     {
         Span<int> data = stackalloc int[3];
+        data.Clear();
+
         var hasLoc = false;
         var hasXYZ = false;
         var hasMap = false;

diff --git a/Projects/Server/Mobiles/Mobile.cs b/Projects/Server/Mobiles/Mobile.cs
@@ -2721,9 +2721,9 @@ public virtual void ProcessDelta()
         Span<byte> facialHairPacket = stackalloc byte[facialHairLength].InitializePacket();
 
         const int cacheLength = OutgoingMobilePackets.MobileMovingPacketCacheByteLength;
-        const int width = OutgoingMobilePackets.MobileMovingPacketLength;
 
-        var mobileMovingCache = stackalloc byte[cacheLength].InitializePackets(width);
+        Span<byte> mobileMovingCache = stackalloc byte[cacheLength];
+        mobileMovingCache.Clear();
 
         var ourState = m_NetState;
 
@@ -4369,9 +4369,9 @@ public virtual bool Move(Direction d)
             if (moveClientQueue.Count > 0)
             {
                 const int cacheLength = OutgoingMobilePackets.MobileMovingPacketCacheByteLength;
-                const int width = OutgoingMobilePackets.MobileMovingPacketLength;
 
-                var mobileMovingCache = stackalloc byte[cacheLength].InitializePackets(width);
+                Span<byte> mobileMovingCache = stackalloc byte[cacheLength];
+                mobileMovingCache.Clear();
 
                 while (moveClientQueue.Count > 0)
                 {
@@ -6923,8 +6923,14 @@ public void SendEverything()
 
                 if (ns.StygianAbyss)
                 {
-                    ns.SendMobileHealthbar(m, Healthbar.Poison);
-                    ns.SendMobileHealthbar(m, Healthbar.Yellow);
+                    if (m.Blessed || m.YellowHealthbar)
+                    {
+                        ns.SendMobileHealthbar(m, Healthbar.Yellow);
+                    }
+                    else if (m.Poisoned)
+                    {
+                        ns.SendMobileHealthbar(m, Healthbar.Poison);
+                    }
                 }
 
                 if (m.IsDeadBondedPet)

diff --git a/Projects/Server/Module.cs b/Projects/Server/Module.cs
@@ -0,0 +1,4 @@
+using System.Runtime.CompilerServices;
+
+// Skips initializing stackalloc with zeros.
+[module: SkipLocalsInit]
diff --git a/Projects/Server/Network/PacketUtilities.cs b/Projects/Server/Network/PacketUtilities.cs
@@ -35,24 +35,7 @@ public static void WritePacketLength(this ref SpanWriter writer)
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static Span<byte> InitializePacket(this Span<byte> buffer)
     {
-#if NO_LOCAL_INIT
-            if (buffer != null)
-            {
-                buffer[0] = 0;
-            }
-#endif
-        return buffer;
-    }
-
-    [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    public static Span<byte> InitializePackets(this Span<byte> buffer, int width)
-    {
-#if NO_LOCAL_INIT
-            for (var i = 0; i < buffer.Length; i += width)
-            {
-                buffer[i] = 0;
-            }
-#endif
+        buffer[0] = 0;
         return buffer;
     }
 }
diff --git a/Projects/Server/Network/Packets/OutgoingEntityPackets.cs b/Projects/Server/Network/Packets/OutgoingEntityPackets.cs
@@ -15,7 +15,10 @@
 
 using System;
 using System.Buffers;
+using System.Buffers.Binary;
+using System.Runtime.CompilerServices;
 using Server.Items;
+using Server.Text;
 
 namespace Server.Network;
 
@@ -25,6 +28,7 @@ public static class OutgoingEntityPackets
     public const int RemoveEntityLength = 5;
     public const int MaxWorldEntityPacketLength = 26;
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static void CreateOPLInfo(Span<byte> buffer, Item item) =>
         CreateOPLInfo(buffer, item.Serial, item.PropertyList.Hash);
 
@@ -41,6 +45,7 @@ public static void CreateOPLInfo(Span<byte> buffer, Serial serial, int hash)
         writer.Write(hash);
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public static void SendOPLInfo(this NetState ns, IObjectPropertyListEntity obj) =>
         ns.SendOPLInfo(obj.Serial, obj.PropertyList.Hash);
 

diff --git a/Projects/Server/Network/Packets/OutgoingMobilePackets.cs b/Projects/Server/Network/Packets/OutgoingMobilePackets.cs
@@ -604,9 +604,7 @@ public static void SendMobileIncoming(this NetState ns, Mobile beholder, Mobile
         }
 
         Span<bool> layers = stackalloc bool[256];
-#if NO_LOCAL_INIT
-            layers.Clear();
-#endif
+        layers.Clear();
 
         var eq = beheld.Items;
         var maxLength = 23 + (eq.Count + 2) * 9;

diff --git a/Projects/Server/Text/StringHelpers.cs b/Projects/Server/Text/StringHelpers.cs
@@ -82,27 +82,14 @@ public static string Remove(this ReadOnlySpan<char> a, ReadOnlySpan<char> b, Str
             return "";
         }
 
-        Span<char> span = a.Length < 1024 ? stackalloc char[a.Length] : null;
-        char[] chrs;
-        if (span == null)
-        {
-            chrs = STArrayPool<char>.Shared.Rent(a.Length);
-            span = chrs.AsSpan();
-        }
-        else
-        {
-            chrs = null;
-        }
+        char[] chrs = STArrayPool<char>.Shared.Rent(a.Length);
+        var span = chrs.AsSpan(0, a.Length);
 
         a.Remove(b, comparison, span, out var size);
 
         var str = span[..size].ToString();
 
-        if (chrs != null)
-        {
-            STArrayPool<char>.Shared.Return(chrs);
-        }
-
+        STArrayPool<char>.Shared.Return(chrs);
         return str;
     }
 
@@ -114,17 +101,8 @@ public static string Capitalize(this string value)
             return value;
         }
 
-        Span<char> span = value.Length < 1024 ? stackalloc char[value.Length] : null;
-        char[] chrs;
-        if (span == null)
-        {
-            chrs = STArrayPool<char>.Shared.Rent(value.Length);
-            span = chrs.AsSpan();
-        }
-        else
-        {
-            chrs = null;
-        }
+        char[] chrs = STArrayPool<char>.Shared.Rent(value.Length);
+        var span = chrs.AsSpan(0, value.Length);
 
         var sliced = value.AsSpan();
         // Copy over the previous span
@@ -161,11 +139,7 @@ public static string Capitalize(this string value)
 
         var str = span.ToString();
 
-        if (chrs != null)
-        {
-            STArrayPool<char>.Shared.Return(chrs);
-        }
-
+        STArrayPool<char>.Shared.Return(chrs);
         return str;
     }
 

diff --git a/Projects/Server/Utilities/Utility.cs b/Projects/Server/Utilities/Utility.cs
@@ -1001,6 +1001,8 @@ public static T[] RandomSample<T>(this T[] source, int count)
 
         var length = source.Length;
         Span<bool> list = stackalloc bool[length];
+        list.Clear();
+
         var sampleList = new T[count];
 
         var i = 0;
@@ -1025,6 +1027,8 @@ public static List<T> RandomSample<T>(this List<T> source, int count)
 
         var length = source.Count;
         Span<bool> list = stackalloc bool[length];
+        list.Clear();
+
         var sampleList = new List<T>(count);
 
         var i = 0;
@@ -1049,6 +1053,7 @@ public static void RandomSample<T>(this T[] source, int count, List<T> dest)
 
         var length = source.Length;
         Span<bool> list = stackalloc bool[length];
+        list.Clear();
 
         var i = 0;
         do

diff --git a/Projects/UOContent/Engines/Party/PartyPackets.cs b/Projects/UOContent/Engines/Party/PartyPackets.cs
@@ -140,15 +140,14 @@ public static void SendPartyInvitation(this NetState ns, Serial leader)
                 return;
             }
 
-            Span<byte> buffer = stackalloc byte[10];
-            var writer = new SpanWriter(buffer);
+            var writer = new SpanWriter(stackalloc byte[10]);
             writer.Write((byte)0xBF); // Packet ID
             writer.Write((ushort)10);
             writer.Write((ushort)0x06); // Sub-packet
             writer.Write((byte)0x07); // command
             writer.Write(leader);
 
-            ns.Send(buffer);
+            ns.Send(writer.Span);
         }
     }
 }
diff --git a/Projects/UOContent/Engines/Quests/Collector/Items/ImageTypeInfo.cs b/Projects/UOContent/Engines/Quests/Collector/Items/ImageTypeInfo.cs
@@ -86,6 +86,8 @@ public static ImageType[] RandomList(int count)
 
         var length = m_Table.Length;
         Span<bool> list = stackalloc bool[length];
+        list.Clear();
+
         var imageTypes = new ImageType[count];
 
         var i = 0;
@@ -100,4 +102,4 @@ public static ImageType[] RandomList(int count)
 
         return imageTypes;
     }
-}
+}
diff --git a/Projects/UOContent/Items/Bulletin Boards/BulletinBoardPackets.cs b/Projects/UOContent/Items/Bulletin Boards/BulletinBoardPackets.cs
@@ -233,7 +233,7 @@ public static void SendBBMessage(this NetState ns, BaseBulletinBoard board, Bull
 
             Span<byte> textBuffer = stackalloc byte[TextEncoding.UTF8.GetMaxByteCount(longestTextLine)];
 
-            var writer = maxLength > 81920 ? new SpanWriter(maxLength) : new SpanWriter(stackalloc byte[maxLength]);
+            var writer = maxLength > 1024 ? new SpanWriter(maxLength) : new SpanWriter(stackalloc byte[maxLength]);
             writer.Write((byte)0x71); // Packet ID
             writer.Seek(2, SeekOrigin.Current);
             writer.Write((byte)(content ? 0x02 : 0x01)); // Command

diff --git a/Projects/UOContent/Module.cs b/Projects/UOContent/Module.cs
@@ -0,0 +1,4 @@
+using System.Runtime.CompilerServices;
+
+// Skips initializing stackalloc with zeros.
+[module: SkipLocalsInit]
diff --git a/Projects/UOContent/Multis/Boats/BoatPackets.cs b/Projects/UOContent/Multis/Boats/BoatPackets.cs
@@ -106,16 +106,14 @@ public static void SendDisplayBoatHS(this NetState ns, Mobile beholder, BaseBoat
 
         using var builder = new PacketContainerBuilder(stackalloc byte[minLength]);
 
-        Span<byte> buffer = builder.GetSpan(OutgoingEntityPackets.MaxWorldEntityPacketLength);
-
         foreach (var entity in boat.GetMovingEntities(true))
         {
             if (!beholder.CanSee(entity))
             {
                 continue;
             }
 
-            buffer.InitializePacket();
+            Span<byte> buffer = builder.GetSpan(OutgoingEntityPackets.MaxWorldEntityPacketLength).InitializePacket();
             var bytesWritten = OutgoingEntityPackets.CreateWorldEntity(buffer, entity, true);
             builder.Advance(bytesWritten);
         }

diff --git a/Projects/UOContent/Multis/Houses/HousePackets.cs b/Projects/UOContent/Multis/Houses/HousePackets.cs
@@ -106,6 +106,8 @@ public static byte[] CreateHouseDesignStateDetailed(Serial serial, int revision,
             using var planesWriter = new SpanWriter(planeLength * planeCount);
 
             Span<bool> planesUsed = stackalloc bool[9];
+            planesUsed.Clear();
+
             int index;
             var totalPlaneOffsets = 0;
             var totalPlanes = 0;