aspnet / HttpAbstractions

[Archived] HTTP abstractions such as HttpRequest, HttpResponse, and HttpContext, as well as common web utilities. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
382 stars 194 forks source link

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

Closed drieseng closed 5 years ago

drieseng commented 5 years ago

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.

drieseng commented 5 years ago

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

drieseng commented 5 years ago

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

poke commented 5 years ago

@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 commented 5 years ago

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.