From 0a3b2474623e3c4424d159add073b078521a0a6a Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Fri, 18 Jul 2025 12:05:15 -0500 Subject: [PATCH 1/4] [STJ] Only compute PopCount once when topologically sorting Enums Packs the calculated (once per entry) PopCount into the high 32 bits of the long and the original index into the low 32 bits. Then that can just be sorted using the heavily optimized Array.Sort() method. After sorting just extract the low 32 bits as the original array index. As before, we negate the actual _PopCount_ to ensure that `Key`s with more on-bits (e.g. more flags represented) will sort **first**. This trades 2 x O(N log N) [average case] to 2 x O(N^2) [worst case] calls to the `popcount` instruction (or the emulation if NET is not defined) for N **shift-left-32** and **or** + N x **truncate to 32 bits**. It _also_ eliminates the overhead of the `CompareTo` method as it's now a direct `long` low-level compare. --- .../Converters/Value/EnumConverter.cs | 45 +++++++++---------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 2dc0a0aebe23a9..a329ecf04d423a 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -676,44 +676,43 @@ private static EnumFieldInfo[] TopologicalSortEnumFields(EnumFieldInfo[] enumFie return enumFields; } - var indices = new int[enumFields.Length]; + var indices = new long[enumFields.Length]; for (int i = 0; i < enumFields.Length; i++) { - indices[i] = i; + // we want values with more bits set to come first so negate the pop count + int popCount = -PopCount(enumFields[i].Key); + // pack into a long with the pop count in the high bits and the index in the low bits + // this allows us to sort by pop count and then by index in case of ties + indices[i] = ((long)popCount << 32) | (uint)i; } - Array.Sort(indices, (i, j) => GetComparisonKey(i).CompareTo(GetComparisonKey(j))); + Array.Sort(indices); var sortedFields = new EnumFieldInfo[enumFields.Length]; for (int i = 0; i < indices.Length; i++) { - sortedFields[i] = enumFields[indices[i]]; + // extract the index from the long, which is the low bits + // the high bits are the pop count, which we don't need anymore + int index = (int)(uint)indices[i]; + sortedFields[i] = enumFields[index]; } return sortedFields; + } - (int PopCount, int Index) GetComparisonKey(int i) - { - // Sort by descending pop count of the enum value. - // Since Array.Sort isn't a stable algorithm - // append the current index to the comparison key. - return (-PopCount(enumFields[i].Key), i); - } - - static int PopCount(ulong value) - { + private static int PopCount(ulong value) + { #if NET - return (int)ulong.PopCount(value); + return (int)ulong.PopCount(value); #else - int count = 0; - while (value != 0) - { - value &= value - 1; - count++; - } - return count; -#endif + int count = 0; + while (value != 0) + { + value &= value - 1; + count++; } + return count; +#endif } private enum EnumFieldNameKind From 53e5e1060d90616876b4580d2b7889dfdbc30ff1 Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Tue, 22 Jul 2025 12:52:42 -0500 Subject: [PATCH 2/4] PR Feedback Switched to Tuple --- .../Serialization/Converters/Value/EnumConverter.cs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index a329ecf04d423a..4c55383e65f917 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -676,14 +676,11 @@ private static EnumFieldInfo[] TopologicalSortEnumFields(EnumFieldInfo[] enumFie return enumFields; } - var indices = new long[enumFields.Length]; + var indices = new Tuple[enumFields.Length]; for (int i = 0; i < enumFields.Length; i++) { // we want values with more bits set to come first so negate the pop count - int popCount = -PopCount(enumFields[i].Key); - // pack into a long with the pop count in the high bits and the index in the low bits - // this allows us to sort by pop count and then by index in case of ties - indices[i] = ((long)popCount << 32) | (uint)i; + indices[i] = Tuple.Create(-PopCount(enumFields[i].Key), i); } Array.Sort(indices); @@ -691,9 +688,8 @@ private static EnumFieldInfo[] TopologicalSortEnumFields(EnumFieldInfo[] enumFie var sortedFields = new EnumFieldInfo[enumFields.Length]; for (int i = 0; i < indices.Length; i++) { - // extract the index from the long, which is the low bits - // the high bits are the pop count, which we don't need anymore - int index = (int)(uint)indices[i]; + // extract the index from the sorted tuple + int index = indices[i].Item2; sortedFields[i] = enumFields[index]; } From a02ebb95f432ee9cfe879fc5e261d46e398d6cb9 Mon Sep 17 00:00:00 2001 From: Marc Brooks Date: Tue, 22 Jul 2025 14:11:22 -0500 Subject: [PATCH 3/4] PR Feedback Switched to native tuple syntax Co-authored-by: Pranav Senthilnathan --- .../Json/Serialization/Converters/Value/EnumConverter.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 4c55383e65f917..0509421b485db4 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -676,11 +676,11 @@ private static EnumFieldInfo[] TopologicalSortEnumFields(EnumFieldInfo[] enumFie return enumFields; } - var indices = new Tuple[enumFields.Length]; + var indices = new (int negativePopCount, int index)[enumFields.Length]; for (int i = 0; i < enumFields.Length; i++) { // we want values with more bits set to come first so negate the pop count - indices[i] = Tuple.Create(-PopCount(enumFields[i].Key), i); + indices[i] = (-PopCount(enumFields[i].Key), i); } Array.Sort(indices); @@ -689,7 +689,7 @@ private static EnumFieldInfo[] TopologicalSortEnumFields(EnumFieldInfo[] enumFie for (int i = 0; i < indices.Length; i++) { // extract the index from the sorted tuple - int index = indices[i].Item2; + int index = indices[i].index; sortedFields[i] = enumFields[index]; } From d1354cdc75048bc499d5aeca5ea33697562dc3e8 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Wed, 23 Jul 2025 10:23:03 +0300 Subject: [PATCH 4/4] Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs --- .../Text/Json/Serialization/Converters/Value/EnumConverter.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs index 0509421b485db4..93f64da6c31d1f 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs @@ -679,7 +679,8 @@ private static EnumFieldInfo[] TopologicalSortEnumFields(EnumFieldInfo[] enumFie var indices = new (int negativePopCount, int index)[enumFields.Length]; for (int i = 0; i < enumFields.Length; i++) { - // we want values with more bits set to come first so negate the pop count + // We want values with more bits set to come first so negate the pop count. + // Keep the index as a second comparand so that sorting stability is preserved. indices[i] = (-PopCount(enumFields[i].Key), i); }