Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes GetChildKeys more efficient #67186

Merged
merged 14 commits into from
Mar 31, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -29,49 +29,64 @@ public class ConfigurationKeyComparer : IComparer<string>
/// <returns>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.</returns>
public int Compare(string? x, string? y)
{
string[] xParts = x?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>();
string[] yParts = y?.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>();

// Compare each part until we get two parts that are not equal
for (int i = 0; i < Math.Min(xParts.Length, yParts.Length); i++)
if (x is not null && y is not null)
{
x = xParts[i];
y = yParts[i];
if (x!.Contains(ConfigurationPath.KeyDelimiter) && y!.Contains(ConfigurationPath.KeyDelimiter))
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
{
string[] xParts = x!.Split(_keyDelimiterArray, StringSplitOptions.RemoveEmptyEntries);
Copy link
Member

@tarekgh tarekgh Mar 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xParts

We can optimize this more by avoiding the allocations occur because of String.Split. I think this is possible by using String.indexof() then compare substrings by indexes (or even using spans).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @tarekgh.

BTW it's a pity that the code which is consuming this type is using string to represent a complex type and uses indexof/split back and forth.

The index is known here:

int indexOf = key.IndexOf(ConfigurationPath.KeyDelimiter, prefixLength, StringComparison.OrdinalIgnoreCase);

then a substring is performed:

return indexOf < 0 ? key.Substring(prefixLength) : key.Substring(prefixLength, indexOf - prefixLength);

then all the logic is reversed to be able to sort the keys:

results.Sort(ConfigurationKeyComparer.Comparison);

It's just a digression, I don't expect @maryamariyan to fix it in this PR (these types are public)

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]);
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
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;
}
else
{
return compare(x, y);
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
}
}

return x?.Length ?? 0 - y?.Length ?? 0;

static int compare(string? a, string? b)
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
{
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);
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved

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;
}
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,64 @@ public void LoadAndCombineKeyValuePairsFromDifferentConfigurationProviders()
Assert.Null(config["NotExist"]);
}

[Fact]
private void GetChildKeys_CanChainEmptyKeys()
{
var input = new Dictionary<string, string>() { };
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<string> 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<string, string>() { };
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<string> 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()
{
Expand Down