Do not check if key is present before removing item in AuthenticationProperties

[drieseng]

• Do not check if key is present before removing item, this eliminates double lookup.
• Use Nullable<T>.GetValueOrDefault() instead of Nullable<T>.Value when it is known to have a value. See Use Nullable<T>.GetValueOrDefault() instead of Nullable<T>.Value #1063 for more info.
• Add some unit tests.

Eliminating the extra lookup yields the following result:

Before:

Method	Mean	Error	StdDev	Op/s	Gen 0	Allocated
NullValue_KeyPresent	123.84 ns	2.1405 ns	1.8975 ns	8,074,763.9	0.0036	328 B
NullValue_KeyNotPresent	41.63 ns	0.5963 ns	0.5577 ns	24,022,181.2	0.0021	192 B

After:

Method	Mean	Error	StdDev	Op/s	Gen 0	Allocated
NullValue_KeyPresent	107.04 ns	0.9192 ns	0.8598 ns	9,342,589.5	0.0038	328 B
NullValue_KeyNotPresent	39.53 ns	0.5507 ns	0.4882 ns	25,300,109.1	0.0021	192 B

Benchmark is available here.

[poke]

Good idea to test the protected utility methods with a subclass. I hadn’t thought of that!

[drieseng]

@poke That's knowledge you gain for free by aging :-)

[Tratcher]

I do wonder what motivated this change. I would not expect removing items from AuthProperties to be that common.

[drieseng]

@Tratcher I just happened to stumble upon it. I didn't come up in profiling session or anything.

[poke]

@Tratcher I would assume that this was motivated by the Value to ValueOrDefault() change and then the key checks were seen in the code.. 😄

[Tratcher]

Note we don't generally accept changes that are unlikely to have a demonstrable production impact. In this case there's the added benefit that it makes the code simpler.