From 03f850fe03c3168d9459ee0fb6f761f2afb5e6e2 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Tue, 21 Nov 2023 12:18:20 -0800 Subject: [PATCH] fix: Fixes sending packets and sidesteps a major issue with stackalloc 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 --- Directory.Build.props | 1 - .../Json/Converters/Point2DConverter.cs | 6 ++- .../Json/Converters/Point3DConverter.cs | 7 +++- .../Json/Converters/Rectangle3DConverter.cs | 7 +++- .../Json/Converters/WorldLocationConverter.cs | 8 +++- Projects/Server/Mobiles/Mobile.cs | 18 ++++++--- Projects/Server/Module.cs | 4 ++ Projects/Server/Network/PacketUtilities.cs | 19 +--------- .../Network/Packets/OutgoingEntityPackets.cs | 5 +++ .../Network/Packets/OutgoingMobilePackets.cs | 4 +- Projects/Server/Text/StringHelpers.cs | 38 +++---------------- Projects/Server/Utilities/Utility.cs | 5 +++ .../UOContent/Engines/Party/PartyPackets.cs | 5 +-- .../Quests/Collector/Items/ImageTypeInfo.cs | 4 +- .../Bulletin Boards/BulletinBoardPackets.cs | 2 +- Projects/UOContent/Module.cs | 4 ++ .../UOContent/Multis/Boats/BoatPackets.cs | 4 +- .../UOContent/Multis/Houses/HousePackets.cs | 2 + 18 files changed, 67 insertions(+), 76 deletions(-) create mode 100644 Projects/Server/Module.cs create mode 100644 Projects/UOContent/Module.cs diff --git a/Directory.Build.props b/Directory.Build.props index 35456bd02..04a2c63e1 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -27,7 +27,6 @@ UNIX CPU_X64 CPU_ARM64 - NO_LOCAL_INIT MUO $(SolutionDir) false diff --git a/Projects/Server/Json/Converters/Point2DConverter.cs b/Projects/Server/Json/Converters/Point2DConverter.cs index 6cfe81aaf..4b5be99e9 100644 --- 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 { - private Point2D DeserializeArray(ref Utf8JsonReader reader) + private static Point2D DeserializeArray(ref Utf8JsonReader reader) { Span data = stackalloc int[2]; + data.Clear(); var count = 0; while (true) @@ -53,9 +54,10 @@ public class Point2DConverter : JsonConverter return new Point2D(data[0], data[1]); } - private Point2D DeserializeObj(ref Utf8JsonReader reader) + private static Point2D DeserializeObj(ref Utf8JsonReader reader) { Span data = stackalloc int[2]; + data.Clear(); while (true) { diff --git a/Projects/Server/Json/Converters/Point3DConverter.cs b/Projects/Server/Json/Converters/Point3DConverter.cs index 9fa34347d..82d029282 100644 --- 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 { - private Point3D DeserializeArray(ref Utf8JsonReader reader) + private static Point3D DeserializeArray(ref Utf8JsonReader reader) { Span data = stackalloc int[3]; + data.Clear(); + var count = 0; while (true) @@ -53,9 +55,10 @@ public class Point3DConverter : JsonConverter return new Point3D(data[0], data[1], data[2]); } - private Point3D DeserializeObj(ref Utf8JsonReader reader) + private static Point3D DeserializeObj(ref Utf8JsonReader reader) { Span data = stackalloc int[3]; + data.Clear(); while (true) { diff --git a/Projects/Server/Json/Converters/Rectangle3DConverter.cs b/Projects/Server/Json/Converters/Rectangle3DConverter.cs index c93aeb50c..75948ad4c 100644 --- 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 { - private Rectangle3D DeserializeArray(ref Utf8JsonReader reader) + private static Rectangle3D DeserializeArray(ref Utf8JsonReader reader) { Span data = stackalloc int[6]; + data.Clear(); + var count = 0; while (true) @@ -53,9 +55,10 @@ public class Rectangle3DConverter : JsonConverter 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 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 index fce2279a7..d83cd4180 100644 --- a/Projects/Server/Json/Converters/WorldLocationConverter.cs +++ b/Projects/Server/Json/Converters/WorldLocationConverter.cs @@ -24,9 +24,11 @@ public class WorldLocationConverter : JsonConverter private static Point3DConverter _point3DConverter; private static MapConverter _mapConverter; - private WorldLocation DeserializeArray(ref Utf8JsonReader reader) + private static WorldLocation DeserializeArray(ref Utf8JsonReader reader) { Span data = stackalloc int[3]; + data.Clear(); + var count = 0; var hasMap = false; Map map = null; @@ -77,9 +79,11 @@ public class WorldLocationConverter : JsonConverter 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 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 index d7336a169..632dc7507 100644 --- a/Projects/Server/Mobiles/Mobile.cs +++ b/Projects/Server/Mobiles/Mobile.cs @@ -2721,9 +2721,9 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro Span facialHairPacket = stackalloc byte[facialHairLength].InitializePacket(); const int cacheLength = OutgoingMobilePackets.MobileMovingPacketCacheByteLength; - const int width = OutgoingMobilePackets.MobileMovingPacketLength; - var mobileMovingCache = stackalloc byte[cacheLength].InitializePackets(width); + Span mobileMovingCache = stackalloc byte[cacheLength]; + mobileMovingCache.Clear(); var ourState = m_NetState; @@ -4369,9 +4369,9 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro if (moveClientQueue.Count > 0) { const int cacheLength = OutgoingMobilePackets.MobileMovingPacketCacheByteLength; - const int width = OutgoingMobilePackets.MobileMovingPacketLength; - var mobileMovingCache = stackalloc byte[cacheLength].InitializePackets(width); + Span mobileMovingCache = stackalloc byte[cacheLength]; + mobileMovingCache.Clear(); while (moveClientQueue.Count > 0) { @@ -6923,8 +6923,14 @@ public partial class Mobile : IHued, IComparable, ISpawnable, IObjectPro 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 new file mode 100644 index 000000000..2de8954a4 --- /dev/null +++ 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 index 7e6bc5f55..7c49a1c59 100644 --- a/Projects/Server/Network/PacketUtilities.cs +++ b/Projects/Server/Network/PacketUtilities.cs @@ -35,24 +35,7 @@ public static class PacketUtilities [MethodImpl(MethodImplOptions.AggressiveInlining)] public static Span InitializePacket(this Span buffer) { -#if NO_LOCAL_INIT - if (buffer != null) - { - buffer[0] = 0; - } -#endif - return buffer; - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static Span InitializePackets(this Span 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 index 50618bc08..092331277 100644 --- 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 buffer, Item item) => CreateOPLInfo(buffer, item.Serial, item.PropertyList.Hash); @@ -41,6 +45,7 @@ public static class OutgoingEntityPackets 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 index 51056afbc..669aa5a95 100644 --- a/Projects/Server/Network/Packets/OutgoingMobilePackets.cs +++ b/Projects/Server/Network/Packets/OutgoingMobilePackets.cs @@ -604,9 +604,7 @@ public static class OutgoingMobilePackets } Span 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 index c908b20f5..c84364c38 100644 --- a/Projects/Server/Text/StringHelpers.cs +++ b/Projects/Server/Text/StringHelpers.cs @@ -82,27 +82,14 @@ public static class StringHelpers return ""; } - Span span = a.Length < 1024 ? stackalloc char[a.Length] : null; - char[] chrs; - if (span == null) - { - chrs = STArrayPool.Shared.Rent(a.Length); - span = chrs.AsSpan(); - } - else - { - chrs = null; - } + char[] chrs = STArrayPool.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.Shared.Return(chrs); - } - + STArrayPool.Shared.Return(chrs); return str; } @@ -114,17 +101,8 @@ public static class StringHelpers return value; } - Span span = value.Length < 1024 ? stackalloc char[value.Length] : null; - char[] chrs; - if (span == null) - { - chrs = STArrayPool.Shared.Rent(value.Length); - span = chrs.AsSpan(); - } - else - { - chrs = null; - } + char[] chrs = STArrayPool.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 class StringHelpers var str = span.ToString(); - if (chrs != null) - { - STArrayPool.Shared.Return(chrs); - } - + STArrayPool.Shared.Return(chrs); return str; } diff --git a/Projects/Server/Utilities/Utility.cs b/Projects/Server/Utilities/Utility.cs index de595be33..1296a44ab 100644 --- a/Projects/Server/Utilities/Utility.cs +++ b/Projects/Server/Utilities/Utility.cs @@ -1001,6 +1001,8 @@ public static class Utility var length = source.Length; Span list = stackalloc bool[length]; + list.Clear(); + var sampleList = new T[count]; var i = 0; @@ -1025,6 +1027,8 @@ public static class Utility var length = source.Count; Span list = stackalloc bool[length]; + list.Clear(); + var sampleList = new List(count); var i = 0; @@ -1049,6 +1053,7 @@ public static class Utility var length = source.Length; Span 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 index d792ffb3f..b27162c4a 100644 --- a/Projects/UOContent/Engines/Party/PartyPackets.cs +++ b/Projects/UOContent/Engines/Party/PartyPackets.cs @@ -140,15 +140,14 @@ namespace Server.Engines.PartySystem return; } - Span 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 index 35bd00a57..188b6a353 100644 --- a/Projects/UOContent/Engines/Quests/Collector/Items/ImageTypeInfo.cs +++ b/Projects/UOContent/Engines/Quests/Collector/Items/ImageTypeInfo.cs @@ -86,6 +86,8 @@ public class ImageTypeInfo var length = m_Table.Length; Span list = stackalloc bool[length]; + list.Clear(); + var imageTypes = new ImageType[count]; var i = 0; @@ -100,4 +102,4 @@ public class ImageTypeInfo return imageTypes; } -} \ No newline at end of file +} diff --git a/Projects/UOContent/Items/Bulletin Boards/BulletinBoardPackets.cs b/Projects/UOContent/Items/Bulletin Boards/BulletinBoardPackets.cs index ec06978ad..c6e45d1bd 100644 --- a/Projects/UOContent/Items/Bulletin Boards/BulletinBoardPackets.cs +++ b/Projects/UOContent/Items/Bulletin Boards/BulletinBoardPackets.cs @@ -233,7 +233,7 @@ namespace Server.Network Span 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 new file mode 100644 index 000000000..2de8954a4 --- /dev/null +++ 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 index 3c48ebe63..c5512f225 100644 --- a/Projects/UOContent/Multis/Boats/BoatPackets.cs +++ b/Projects/UOContent/Multis/Boats/BoatPackets.cs @@ -106,8 +106,6 @@ public static class BoatPackets using var builder = new PacketContainerBuilder(stackalloc byte[minLength]); - Span buffer = builder.GetSpan(OutgoingEntityPackets.MaxWorldEntityPacketLength); - foreach (var entity in boat.GetMovingEntities(true)) { if (!beholder.CanSee(entity)) @@ -115,7 +113,7 @@ public static class BoatPackets continue; } - buffer.InitializePacket(); + Span 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 index a2bb5f219..25dae3088 100644 --- a/Projects/UOContent/Multis/Houses/HousePackets.cs +++ b/Projects/UOContent/Multis/Houses/HousePackets.cs @@ -106,6 +106,8 @@ namespace Server.Multis using var planesWriter = new SpanWriter(planeLength * planeCount); Span planesUsed = stackalloc bool[9]; + planesUsed.Clear(); + int index; var totalPlaneOffsets = 0; var totalPlanes = 0;