From b2e948f857399df3d7eab32f8eebf67e9b41e47d Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 25 Mar 2022 22:48:37 -0700 Subject: [PATCH 01/14] Makes GetChildKeys more efficient --- .../src/ConfigurationKeyComparer.cs | 66 +++++++++++++------ .../tests/ConfigurationTest.cs | 58 ++++++++++++++++ 2 files changed, 105 insertions(+), 19 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 00cca781d47f46..957fffa5fa4d01 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -29,29 +29,65 @@ public class ConfigurationKeyComparer : IComparer /// Less than 0 if x is less than y, 0 if x is equal to y and greater than 0 if x is greater than y. public int Compare(string? x, string? y) { - string[] xParts = x?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty(); - string[] yParts = y?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty(); + if (string.IsNullOrWhiteSpace(x)) + { + if (string.IsNullOrWhiteSpace(y)) + { + return 0; + } + + return -y!.Length; + } + else if (string.IsNullOrWhiteSpace(y)) + { + return x!.Length; + } + + int result; + if (!x!.Contains(ConfigurationPath.KeyDelimiter) || !y!.Contains(ConfigurationPath.KeyDelimiter)) + { + result = compare(x, y); + if (result != 0) + { + // One of them is different + return result; + } + } + + string[] xParts = x.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + string[] yParts = y.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); // Compare each part until we get two parts that are not equal for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) { - x = xParts[i]; - y = yParts[i]; + result = compare(xParts[i], yParts[i]); + if (result != 0) + { + // One of them is different + return result; + } + } + // If we get here, the common parts are equal. + // If they are of the same length, then they are totally identical + return xParts.Length - yParts.Length; + + static int compare(string? a, string? b) + { int value1 = 0; int value2 = 0; - bool xIsInt = x != null && int.TryParse(x, out value1); - bool yIsInt = y != null && int.TryParse(y, out value2); + bool aIsInt = a != null && int.TryParse(a, out value1); + bool bIsInt = b != null && int.TryParse(b, out value2); int result; - if (!xIsInt && !yIsInt) + if (!aIsInt && !bIsInt) { // Both are strings - result = string.Compare(x, y, StringComparison.OrdinalIgnoreCase); + result = string.Compare(a, b, StringComparison.OrdinalIgnoreCase); } - else if (xIsInt && yIsInt) + else if (aIsInt && bIsInt) { // Both are int result = value1 - value2; @@ -59,19 +95,11 @@ public int Compare(string? x, string? y) else { // Only one of them is int - result = xIsInt ? -1 : 1; + result = aIsInt ? -1 : 1; } - if (result != 0) - { - // One of them is different - return result; - } + return result; } - - // If we get here, the common parts are equal. - // If they are of the same length, then they are totally identical - return xParts.Length - yParts.Length; } } } diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs index 70a1abf218a10d..7d0df82f585618 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationTest.cs @@ -59,6 +59,64 @@ public void LoadAndCombineKeyValuePairsFromDifferentConfigurationProviders() Assert.Null(config["NotExist"]); } + [Fact] + private void GetChildKeys_CanChainEmptyKeys() + { + var input = new Dictionary() { }; + for (int i = 0; i < 1000; i++) + { + input.Add(new string(' ', i), string.Empty); + } + + IConfigurationRoot configurationRoot = new ConfigurationBuilder() + .Add(new MemoryConfigurationSource + { + InitialData = input + }) + .Build(); + + var chainedConfigurationSource = new ChainedConfigurationSource + { + Configuration = configurationRoot, + ShouldDisposeConfiguration = false, + }; + + var chainedConfiguration = new ChainedConfigurationProvider(chainedConfigurationSource); + IEnumerable childKeys = chainedConfiguration.GetChildKeys(new string[0], null); + Assert.Equal(1000, childKeys.Count()); + Assert.Equal(string.Empty, childKeys.First()); + Assert.Equal(999, childKeys.Last().Length); + } + + [Fact] + private void GetChildKeys_CanChainKeyWithNoDelimiter() + { + var input = new Dictionary() { }; + for (int i = 1000; i < 2000; i++) + { + input.Add(i.ToString(), string.Empty); + } + + IConfigurationRoot configurationRoot = new ConfigurationBuilder() + .Add(new MemoryConfigurationSource + { + InitialData = input + }) + .Build(); + + var chainedConfigurationSource = new ChainedConfigurationSource + { + Configuration = configurationRoot, + ShouldDisposeConfiguration = false, + }; + + var chainedConfiguration = new ChainedConfigurationProvider(chainedConfigurationSource); + IEnumerable childKeys = chainedConfiguration.GetChildKeys(new string[0], null); + Assert.Equal(1000, childKeys.Count()); + Assert.Equal("1000", childKeys.First()); + Assert.Equal("1999", childKeys.Last()); + } + [Fact] public void CanChainConfiguration() { From da96dd93b64e220d3d1c97cca0e20ac6b464df8e Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 25 Mar 2022 23:58:52 -0700 Subject: [PATCH 02/14] Remove redundant else --- .../src/ConfigurationKeyComparer.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 957fffa5fa4d01..569162a6ef6005 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -38,7 +38,8 @@ public int Compare(string? x, string? y) return -y!.Length; } - else if (string.IsNullOrWhiteSpace(y)) + + if (string.IsNullOrWhiteSpace(y)) { return x!.Length; } From f35f0c5ec7f62ceb539b1ca94a970c09d1eb647b Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Sat, 26 Mar 2022 01:28:22 -0700 Subject: [PATCH 03/14] Optimize for split case --- .../src/ConfigurationKeyComparer.cs | 58 +++++++------------ 1 file changed, 22 insertions(+), 36 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 569162a6ef6005..b2fe7f245e8257 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -29,49 +29,35 @@ public class ConfigurationKeyComparer : IComparer /// Less than 0 if x is less than y, 0 if x is equal to y and greater than 0 if x is greater than y. public int Compare(string? x, string? y) { - if (string.IsNullOrWhiteSpace(x)) + if (x is not null && y is not null) { - if (string.IsNullOrWhiteSpace(y)) + if (x!.Contains(ConfigurationPath.KeyDelimiter) && y!.Contains(ConfigurationPath.KeyDelimiter)) { - return 0; + string[] xParts = x!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + string[] yParts = y!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + + // Compare each part until we get two parts that are not equal + for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) + { + int result = compare(xParts[i], yParts[i]); + if (result != 0) + { + // One of them is different + return result; + } + } + + // If we get here, the common parts are equal. + // If they are of the same length, then they are totally identical + return xParts.Length - yParts.Length; } - - return -y!.Length; - } - - if (string.IsNullOrWhiteSpace(y)) - { - return x!.Length; - } - - int result; - if (!x!.Contains(ConfigurationPath.KeyDelimiter) || !y!.Contains(ConfigurationPath.KeyDelimiter)) - { - result = compare(x, y); - if (result != 0) - { - // One of them is different - return result; - } - } - - string[] xParts = x.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); - string[] yParts = y.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); - - // Compare each part until we get two parts that are not equal - for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) - { - result = compare(xParts[i], yParts[i]); - if (result != 0) + else { - // One of them is different - return result; + return compare(x, y); } } - // If we get here, the common parts are equal. - // If they are of the same length, then they are totally identical - return xParts.Length - yParts.Length; + return x?.Length ?? 0 - y?.Length ?? 0; static int compare(string? a, string? b) { From 006b747fdf7180b12d4a7fe13156954aca3fa2d5 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 28 Mar 2022 12:24:12 -0400 Subject: [PATCH 04/14] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- .../src/ConfigurationKeyComparer.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index b2fe7f245e8257..a0dec964443609 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -31,7 +31,7 @@ public int Compare(string? x, string? y) { if (x is not null && y is not null) { - if (x!.Contains(ConfigurationPath.KeyDelimiter) && y!.Contains(ConfigurationPath.KeyDelimiter)) + if (x.Contains(ConfigurationPath.KeyDelimiter) && y!.Contains(ConfigurationPath.KeyDelimiter)) { string[] xParts = x!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); string[] yParts = y!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); @@ -39,7 +39,7 @@ public int Compare(string? x, string? y) // Compare each part until we get two parts that are not equal for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) { - int result = compare(xParts[i], yParts[i]); + int result = Compare(xParts[i], yParts[i]); if (result != 0) { // One of them is different @@ -53,19 +53,19 @@ public int Compare(string? x, string? y) } else { - return compare(x, y); + return Compare(x, y); } } return x?.Length ?? 0 - y?.Length ?? 0; - static int compare(string? a, string? b) + static int Compare(string? a, string? b) { int value1 = 0; int value2 = 0; - bool aIsInt = a != null && int.TryParse(a, out value1); - bool bIsInt = b != null && int.TryParse(b, out value2); + bool aIsInt = int.TryParse(a, out value1); + bool bIsInt = int.TryParse(b, out value2); int result; From f6097344a385ecf07aa294513ddcdd3b05fcd671 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 28 Mar 2022 09:36:37 -0700 Subject: [PATCH 05/14] Fix compile after auto resolve --- .../src/ConfigurationKeyComparer.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index a0dec964443609..e0dec763ded5b6 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -61,11 +61,8 @@ public int Compare(string? x, string? y) static int Compare(string? a, string? b) { - int value1 = 0; - int value2 = 0; - - bool aIsInt = int.TryParse(a, out value1); - bool bIsInt = int.TryParse(b, out value2); + bool aIsInt = int.TryParse(a, out int value1); + bool bIsInt = int.TryParse(b, out int value2); int result; From ec3c61f49d58a5accf49d5713ccbd941166e9b44 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 28 Mar 2022 12:54:30 -0700 Subject: [PATCH 06/14] Using IndexOf rather than Split --- .../src/ConfigurationKeyComparer.cs | 57 +++++++++++++++---- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index e0dec763ded5b6..95d69dffd4a9c0 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -31,45 +31,78 @@ public int Compare(string? x, string? y) { if (x is not null && y is not null) { - if (x.Contains(ConfigurationPath.KeyDelimiter) && y!.Contains(ConfigurationPath.KeyDelimiter)) + if (x.Contains(ConfigurationPath.KeyDelimiter) && y.Contains(ConfigurationPath.KeyDelimiter)) { - string[] xParts = x!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); - string[] yParts = y!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + int xIndex = 0; + int yIndex = 0; // Compare each part until we get two parts that are not equal - for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++) + while (xIndex < x.Length && yIndex < y.Length) { - int result = Compare(xParts[i], yParts[i]); - if (result != 0) + int nextXIndex = x.IndexOf(ConfigurationPath.KeyDelimiter, xIndex); + int nextYIndex = y.IndexOf(ConfigurationPath.KeyDelimiter, yIndex); + + ReadOnlySpan xSpan; + if (nextXIndex >= 0) + { + xSpan = x.AsSpan().Slice(xIndex, nextXIndex - xIndex); + xIndex = nextXIndex + 1; + } + else { - // One of them is different - return result; + xSpan = x.AsSpan().Slice(xIndex, x.Length - xIndex); + xIndex = x.Length; + } + + ReadOnlySpan ySpan; + if (nextYIndex >= 0) + { + ySpan = y.AsSpan().Slice(yIndex, nextYIndex - yIndex); + yIndex = nextYIndex + 1; + } + else + { + ySpan = y.AsSpan().Slice(yIndex, y.Length - yIndex); + yIndex = y.Length; + } + + int compareResult = Compare(xSpan, ySpan); + if (compareResult != 0) + { + return compareResult; } } + string[] xParts = x.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + string[] yParts = y.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + // If we get here, the common parts are equal. // If they are of the same length, then they are totally identical return xParts.Length - yParts.Length; } else { - return Compare(x, y); + return Compare(x.AsSpan(), y.AsSpan()); } } return x?.Length ?? 0 - y?.Length ?? 0; - static int Compare(string? a, string? b) + static int Compare(ReadOnlySpan a, ReadOnlySpan b) { +#if NETCOREAPP bool aIsInt = int.TryParse(a, out int value1); bool bIsInt = int.TryParse(b, out int value2); - +#else + bool aIsInt = int.TryParse(a.ToString(), out int value1); + bool bIsInt = int.TryParse(b.ToString(), out int value2); +#endif int result; if (!aIsInt && !bIsInt) { // Both are strings - result = string.Compare(a, b, StringComparison.OrdinalIgnoreCase); + result = a.CompareTo(b, StringComparison.OrdinalIgnoreCase); } else if (aIsInt && bIsInt) { From d6cf5972d7023bd70be0a8c3f52102b959726f66 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 29 Mar 2022 20:39:10 -0700 Subject: [PATCH 07/14] Removes Split in favor of IndexOf - Also fixing a regression from last commit + adds test --- .../src/ConfigurationKeyComparer.cs | 126 +++++++++++------- .../tests/ConfigurationPathComparerTest.cs | 16 +++ 2 files changed, 94 insertions(+), 48 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 95d69dffd4a9c0..3b48caf2d6022e 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using Microsoft.Extensions.Primitives; namespace Microsoft.Extensions.Configuration { @@ -11,7 +12,7 @@ namespace Microsoft.Extensions.Configuration /// public class ConfigurationKeyComparer : IComparer { - private static readonly string[] _keyDelimiterArray = new[] { ConfigurationPath.KeyDelimiter }; + private static readonly char s_keyDelimiter = ConfigurationPath.KeyDelimiter[0]; /// /// The default instance. @@ -29,64 +30,93 @@ public class ConfigurationKeyComparer : IComparer /// Less than 0 if x is less than y, 0 if x is equal to y and greater than 0 if x is greater than y. public int Compare(string? x, string? y) { - if (x is not null && y is not null) + if (x == null || y == null) { - if (x.Contains(ConfigurationPath.KeyDelimiter) && y.Contains(ConfigurationPath.KeyDelimiter)) + return (x is null ? 0 : GetPartsCount(x)) - (y is null ? 0 : GetPartsCount(y)); + } + + int xIndex = 0; + int yIndex = 0; + + // Compare each part until we get two parts that are not equal + while (xIndex < x.Length && yIndex < y.Length) + { + if (x[xIndex] == s_keyDelimiter) { - int xIndex = 0; - int yIndex = 0; + xIndex++; + continue; + } - // Compare each part until we get two parts that are not equal - while (xIndex < x.Length && yIndex < y.Length) - { - int nextXIndex = x.IndexOf(ConfigurationPath.KeyDelimiter, xIndex); - int nextYIndex = y.IndexOf(ConfigurationPath.KeyDelimiter, yIndex); - - ReadOnlySpan xSpan; - if (nextXIndex >= 0) - { - xSpan = x.AsSpan().Slice(xIndex, nextXIndex - xIndex); - xIndex = nextXIndex + 1; - } - else - { - xSpan = x.AsSpan().Slice(xIndex, x.Length - xIndex); - xIndex = x.Length; - } - - ReadOnlySpan ySpan; - if (nextYIndex >= 0) - { - ySpan = y.AsSpan().Slice(yIndex, nextYIndex - yIndex); - yIndex = nextYIndex + 1; - } - else - { - ySpan = y.AsSpan().Slice(yIndex, y.Length - yIndex); - yIndex = y.Length; - } - - int compareResult = Compare(xSpan, ySpan); - if (compareResult != 0) - { - return compareResult; - } - } + if (y[yIndex] == s_keyDelimiter) + { + yIndex++; + continue; + } - string[] xParts = x.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); - string[] yParts = y.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries); + ReadOnlySpan xSpan; + int nextXIndex = x.IndexOf(s_keyDelimiter, xIndex); + if (nextXIndex >= 0) + { + xSpan = x.AsSpan().Slice(xIndex, nextXIndex - xIndex); + xIndex = nextXIndex + 1; + } + else + { + xSpan = x.AsSpan().Slice(xIndex, x.Length - xIndex); + xIndex = x.Length; + } - // If we get here, the common parts are equal. - // If they are of the same length, then they are totally identical - return xParts.Length - yParts.Length; + ReadOnlySpan ySpan; + int nextYIndex = y.IndexOf(s_keyDelimiter, yIndex); + if (nextYIndex >= 0) + { + ySpan = y.AsSpan().Slice(yIndex, nextYIndex - yIndex); + yIndex = nextYIndex + 1; } else { - return Compare(x.AsSpan(), y.AsSpan()); + ySpan = y.AsSpan().Slice(yIndex, y.Length - yIndex); + yIndex = y.Length; } + + int compareResult = Compare(xSpan, ySpan); + if (compareResult != 0) + { + return compareResult; + } + } + + if (xIndex >= x.Length) + { + return yIndex >= y.Length ? 0 : -GetPartsCount(y, yIndex); } - return x?.Length ?? 0 - y?.Length ?? 0; + return GetPartsCount(x, xIndex); + + static int GetPartsCount(string s, int sliceIndex = 0) + { + ReadOnlySpan a = s.AsSpan().Slice(sliceIndex, s.Length - sliceIndex); + int count = 0, aIndex = 0; + while (aIndex < a.Length) + { + int nextAIndex = a.Slice(aIndex).IndexOf(s_keyDelimiter); + if (nextAIndex < 0) + { + return count + 1; + } + + if (a[aIndex] == s_keyDelimiter) + { + aIndex++; + continue; + } + + aIndex += nextAIndex + 1; + count++; + } + + return count; + } static int Compare(ReadOnlySpan a, ReadOnlySpan b) { diff --git a/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationPathComparerTest.cs b/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationPathComparerTest.cs index 159ba1d4a75cdf..a61cc806176036 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationPathComparerTest.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/tests/ConfigurationPathComparerTest.cs @@ -14,6 +14,8 @@ public void CompareWithNull() ComparerTest(null, null, 0); ComparerTest(null, "a", -1); ComparerTest("b", null, 1); + ComparerTest(null, "a:b", -1); + ComparerTest(null, "a:b:c", -1); } [Fact] @@ -32,6 +34,20 @@ public void CompareWithDifferentLengths() ComparerTest("aa", "a", 1); } + [Fact] + public void CompareWithEmpty() + { + ComparerTest(":", "", 0); + ComparerTest(":", "::", 0); + ComparerTest(null, "", 0); + ComparerTest(":", null, 0); + ComparerTest("::", null, 0); + ComparerTest(" : : ", null, 1); + ComparerTest("b: :a", "b::a", -1); + ComparerTest("b:\t:a", "b::a", -1); + ComparerTest("b::a: ", "b::a:", 1); + } + [Fact] public void CompareWithLetters() { From 59e76b14c853b49e881a2b9314f766ffc3710bbb Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 00:45:57 -0700 Subject: [PATCH 08/14] Improve readability using span for looping --- .../src/ConfigurationKeyComparer.cs | 58 ++++++++----------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 3b48caf2d6022e..e28d58f0556b2c 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -30,72 +30,60 @@ public class ConfigurationKeyComparer : IComparer /// Less than 0 if x is less than y, 0 if x is equal to y and greater than 0 if x is greater than y. public int Compare(string? x, string? y) { + ReadOnlySpan xSpan = x.AsSpan(); + ReadOnlySpan ySpan = y.AsSpan(); + if (x == null || y == null) { - return (x is null ? 0 : GetPartsCount(x)) - (y is null ? 0 : GetPartsCount(y)); + return (x is null ? 0 : CountPartsIn(xSpan)) - (y is null ? 0 : CountPartsIn(ySpan)); } - int xIndex = 0; - int yIndex = 0; - // Compare each part until we get two parts that are not equal - while (xIndex < x.Length && yIndex < y.Length) + while (!xSpan.IsEmpty && !ySpan.IsEmpty) { - if (x[xIndex] == s_keyDelimiter) + if (xSpan[0] == s_keyDelimiter) { - xIndex++; + xSpan = xSpan.Slice(1); continue; } - if (y[yIndex] == s_keyDelimiter) + if (ySpan[0] == s_keyDelimiter) { - yIndex++; + ySpan = ySpan.Slice(1); continue; } - ReadOnlySpan xSpan; - int nextXIndex = x.IndexOf(s_keyDelimiter, xIndex); - if (nextXIndex >= 0) - { - xSpan = x.AsSpan().Slice(xIndex, nextXIndex - xIndex); - xIndex = nextXIndex + 1; - } - else + int nextXIndex = xSpan.IndexOf(s_keyDelimiter); + if (nextXIndex < 0) { - xSpan = x.AsSpan().Slice(xIndex, x.Length - xIndex); - xIndex = x.Length; + nextXIndex = xSpan.Length; } - ReadOnlySpan ySpan; - int nextYIndex = y.IndexOf(s_keyDelimiter, yIndex); - if (nextYIndex >= 0) + int nextYIndex = ySpan.IndexOf(s_keyDelimiter); + if (nextYIndex < 0) { - ySpan = y.AsSpan().Slice(yIndex, nextYIndex - yIndex); - yIndex = nextYIndex + 1; - } - else - { - ySpan = y.AsSpan().Slice(yIndex, y.Length - yIndex); - yIndex = y.Length; + nextYIndex = ySpan.Length; } - int compareResult = Compare(xSpan, ySpan); + int compareResult = Compare(xSpan.Slice(0, nextXIndex), ySpan.Slice(0, nextYIndex)); if (compareResult != 0) { return compareResult; } + + xSpan = xSpan.Slice(nextXIndex); + ySpan = ySpan.Slice(nextYIndex); } - if (xIndex >= x.Length) + if (xSpan.IsEmpty) { - return yIndex >= y.Length ? 0 : -GetPartsCount(y, yIndex); + return ySpan.IsEmpty ? 0 : -CountPartsIn(ySpan); } - return GetPartsCount(x, xIndex); + return CountPartsIn(xSpan); - static int GetPartsCount(string s, int sliceIndex = 0) + static int CountPartsIn(ReadOnlySpan a) { - ReadOnlySpan a = s.AsSpan().Slice(sliceIndex, s.Length - sliceIndex); int count = 0, aIndex = 0; while (aIndex < a.Length) { From 2c181bf927fd55e1f7a349ab89a0c36a36ec7bfe Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 01:29:05 -0700 Subject: [PATCH 09/14] Removes unused using --- .../src/ConfigurationKeyComparer.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index e28d58f0556b2c..6bd31c5c5dfa7b 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using Microsoft.Extensions.Primitives; namespace Microsoft.Extensions.Configuration { From 136a52416d26f51af7fab6e0596eeb0d6c614b62 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 01:52:31 -0700 Subject: [PATCH 10/14] Further code cleanup --- .../src/ConfigurationKeyComparer.cs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 6bd31c5c5dfa7b..b3e220604d50ad 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -52,26 +52,26 @@ public int Compare(string? x, string? y) continue; } - int nextXIndex = xSpan.IndexOf(s_keyDelimiter); - if (nextXIndex < 0) + int xSegmentLength = xSpan.IndexOf(s_keyDelimiter); + if (xSegmentLength < 0) { - nextXIndex = xSpan.Length; + xSegmentLength = xSpan.Length; } - int nextYIndex = ySpan.IndexOf(s_keyDelimiter); - if (nextYIndex < 0) + int ySegmentLength = ySpan.IndexOf(s_keyDelimiter); + if (ySegmentLength < 0) { - nextYIndex = ySpan.Length; + ySegmentLength = ySpan.Length; } - int compareResult = Compare(xSpan.Slice(0, nextXIndex), ySpan.Slice(0, nextYIndex)); + int compareResult = Compare(xSpan.Slice(0, xSegmentLength), ySpan.Slice(0, ySegmentLength)); if (compareResult != 0) { return compareResult; } - xSpan = xSpan.Slice(nextXIndex); - ySpan = ySpan.Slice(nextYIndex); + xSpan = xSpan.Slice(xSegmentLength); + ySpan = ySpan.Slice(ySegmentLength); } if (xSpan.IsEmpty) @@ -83,22 +83,22 @@ public int Compare(string? x, string? y) static int CountPartsIn(ReadOnlySpan a) { - int count = 0, aIndex = 0; - while (aIndex < a.Length) + int count = 0; + while (!a.IsEmpty) { - int nextAIndex = a.Slice(aIndex).IndexOf(s_keyDelimiter); - if (nextAIndex < 0) + int segmentLength = a.IndexOf(s_keyDelimiter); + if (segmentLength < 0) { return count + 1; } - if (a[aIndex] == s_keyDelimiter) + if (a[0] == s_keyDelimiter) { - aIndex++; + a = a.Slice(1); continue; } - aIndex += nextAIndex + 1; + a = a.Slice(segmentLength); count++; } From 012c38db0cba0dae6937c5500bc603e890abf66a Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 10:21:37 -0700 Subject: [PATCH 11/14] Remove CountPartsIn --- .../src/ConfigurationKeyComparer.cs | 49 ++++--------------- 1 file changed, 9 insertions(+), 40 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index b3e220604d50ad..5b739962315e56 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -32,26 +32,12 @@ public int Compare(string? x, string? y) ReadOnlySpan xSpan = x.AsSpan(); ReadOnlySpan ySpan = y.AsSpan(); - if (x == null || y == null) - { - return (x is null ? 0 : CountPartsIn(xSpan)) - (y is null ? 0 : CountPartsIn(ySpan)); - } + xSpan = SkipAheadOnDelimiter(xSpan); + ySpan = SkipAheadOnDelimiter(ySpan); // Compare each part until we get two parts that are not equal while (!xSpan.IsEmpty && !ySpan.IsEmpty) { - if (xSpan[0] == s_keyDelimiter) - { - xSpan = xSpan.Slice(1); - continue; - } - - if (ySpan[0] == s_keyDelimiter) - { - ySpan = ySpan.Slice(1); - continue; - } - int xSegmentLength = xSpan.IndexOf(s_keyDelimiter); if (xSegmentLength < 0) { @@ -72,37 +58,20 @@ public int Compare(string? x, string? y) xSpan = xSpan.Slice(xSegmentLength); ySpan = ySpan.Slice(ySegmentLength); - } - if (xSpan.IsEmpty) - { - return ySpan.IsEmpty ? 0 : -CountPartsIn(ySpan); + xSpan = SkipAheadOnDelimiter(xSpan); + ySpan = SkipAheadOnDelimiter(ySpan); } - return CountPartsIn(xSpan); + return xSpan.IsEmpty ? (ySpan.IsEmpty ? 0 : -1) : 1; - static int CountPartsIn(ReadOnlySpan a) + static ReadOnlySpan SkipAheadOnDelimiter(ReadOnlySpan a) { - int count = 0; - while (!a.IsEmpty) + while (!a.IsEmpty && a[0] == s_keyDelimiter) { - int segmentLength = a.IndexOf(s_keyDelimiter); - if (segmentLength < 0) - { - return count + 1; - } - - if (a[0] == s_keyDelimiter) - { - a = a.Slice(1); - continue; - } - - a = a.Slice(segmentLength); - count++; + a = a.Slice(1); } - - return count; + return a; } static int Compare(ReadOnlySpan a, ReadOnlySpan b) From a423633b702dc9788af71cb05c38b3a077d53ea5 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 14:47:52 -0400 Subject: [PATCH 12/14] Update src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs --- .../src/ConfigurationKeyComparer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 5b739962315e56..9f6acf9bc3bcb8 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -11,7 +11,7 @@ namespace Microsoft.Extensions.Configuration /// public class ConfigurationKeyComparer : IComparer { - private static readonly char s_keyDelimiter = ConfigurationPath.KeyDelimiter[0]; + private const char KeyDelimiter = ':'; /// /// The default instance. From adeecd8b238309c91ad3cd0944df61e73c562de3 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 14:40:41 -0700 Subject: [PATCH 13/14] Apply PR feedback --- .../src/ConfigurationKeyComparer.cs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 9f6acf9bc3bcb8..2f06d453ed8ea3 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -38,36 +38,29 @@ public int Compare(string? x, string? y) // Compare each part until we get two parts that are not equal while (!xSpan.IsEmpty && !ySpan.IsEmpty) { - int xSegmentLength = xSpan.IndexOf(s_keyDelimiter); - if (xSegmentLength < 0) - { - xSegmentLength = xSpan.Length; - } + int xDelimiterIndex = xSpan.IndexOf(KeyDelimiter); + int yDelimiterIndex = ySpan.IndexOf(KeyDelimiter); - int ySegmentLength = ySpan.IndexOf(s_keyDelimiter); - if (ySegmentLength < 0) - { - ySegmentLength = ySpan.Length; - } + int compareResult = Compare( + xDelimiterIndex == -1 ? xSpan : xSpan.Slice(0, xDelimiterIndex), + yDelimiterIndex == -1 ? ySpan : ySpan.Slice(0, yDelimiterIndex)); - int compareResult = Compare(xSpan.Slice(0, xSegmentLength), ySpan.Slice(0, ySegmentLength)); if (compareResult != 0) { return compareResult; } - xSpan = xSpan.Slice(xSegmentLength); - ySpan = ySpan.Slice(ySegmentLength); - - xSpan = SkipAheadOnDelimiter(xSpan); - ySpan = SkipAheadOnDelimiter(ySpan); + xSpan = xDelimiterIndex == -1 ? default : + SkipAheadOnDelimiter(xSpan.Slice(xDelimiterIndex)); + ySpan = yDelimiterIndex == -1 ? default : + SkipAheadOnDelimiter(ySpan.Slice(yDelimiterIndex)); } return xSpan.IsEmpty ? (ySpan.IsEmpty ? 0 : -1) : 1; static ReadOnlySpan SkipAheadOnDelimiter(ReadOnlySpan a) { - while (!a.IsEmpty && a[0] == s_keyDelimiter) + while (!a.IsEmpty && a[0] == KeyDelimiter) { a = a.Slice(1); } From b128f6325108f2e3dd22834d9fef76e37b53695f Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 30 Mar 2022 19:12:18 -0400 Subject: [PATCH 14/14] Apply suggestions from code review Co-authored-by: Eric Erhardt --- .../src/ConfigurationKeyComparer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs index 2f06d453ed8ea3..f00a9ab49f041d 100644 --- a/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs +++ b/src/libraries/Microsoft.Extensions.Configuration/src/ConfigurationKeyComparer.cs @@ -51,9 +51,9 @@ public int Compare(string? x, string? y) } xSpan = xDelimiterIndex == -1 ? default : - SkipAheadOnDelimiter(xSpan.Slice(xDelimiterIndex)); + SkipAheadOnDelimiter(xSpan.Slice(xDelimiterIndex + 1)); ySpan = yDelimiterIndex == -1 ? default : - SkipAheadOnDelimiter(ySpan.Slice(yDelimiterIndex)); + SkipAheadOnDelimiter(ySpan.Slice(yDelimiterIndex + 1)); } return xSpan.IsEmpty ? (ySpan.IsEmpty ? 0 : -1) : 1;