Question: Get rid of NameValuePair _keyChain field in DbConnectionOptions.Common.cs

[TrayanZapryanov]

In DbConnectionOptions there are two quite similar structures to store connection string Key/Value pairs.
The main difference that I can see is only the order.
Is it really critical to show values as passed from user(password replaced with *), or it doesn't matter ?

File: DbConnectionOptions.Common.cs
Fields:
`
private readonly Dictionary<string, string> _parsetable;

internal readonly NameValuePair _keyChain;
`

If I comment _keyChain - it looks to be used only in ExpandAttachDbFileName and ReplacePasswordPwd.
Both can be mimic with _parsetable.

[JRahnama]

@TrayanZapryanov Thanks for bringing this up to our attention. We will look into this issue and get back to you soon.

[TrayanZapryanov]

@Wraith2 Can we move our discussion about caching parsed values here as other 681 is just an issue and can be solved with simple if around ETW log?

Some time ago I saw an lock free LRU in runtime repo: Regex.Cache

Maybe it is the correct structure to be used storing _parsetable/_keyChain per connection string.
At least from comments it fits in our case. Of course we can ask Stephen Toub if similar structure can be reused without any drawbacks here.

[Wraith2]

Give it a try and see how it affects perf. If it's in runtime it's probably ok to pull in a copy here as long as there are no functional changes that require a re-audit of functionality.

[TrayanZapryanov]

I will try to submit a PR tomorrow.

[TrayanZapryanov]

Unfortunatelly I was wrong that allocations came from parsing connection string to key/value pairs. This parsing is done once per connnection string and then cached in DbConnectionFactory by _connectionPoolGroups dictionary.
Still question in this topic is relevant, but I don't think that this will bring any performance benefit.

Once other issues that I've raised (681 and 680) are in, I receive following benchmark results:

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19041.388 (2004/?/20H1)
Intel Core i7-4770 CPU 3.40GHz (Haswell), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.1.302
[Host] : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT
DefaultJob : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Create	273.3 ns	4.94 ns	4.85 ns	0.0572	-	-	240 B
Create_System	221.8 ns	2.86 ns	3.30 ns	0.0629	-	-	264 B

Still there is some regression, but way better that before. Also a bit better in allocations.

[cheenamalhotra]

@TrayanZapryanov

If your proposed changes reduce duplicities or enhance readability, they're good changes.
So do not hesitate, let us know when you have a PR ready, we would review it then.