Is Aspnetcore open to community perf improvements?

[TylerBurnett]

Hello!

I've noticed that the contributing guidelines require a ticket to exist before a PR. I interpret this as you guys are not so inclined to PR's for perf. Is this the case? Do you have guidelines for what circumstances make it ok?

To provide some context for what changes I would propose, please have a look at this diff
main...TylerBurnett:aspnetcore:main

Thank you

[davidfowl]

I've noticed that the contributing guidelines require a ticket to exist before a PR

https://www.igvita.com/2011/12/19/dont-push-your-pull-requests/

If the change is claiming performance improvements, then you should have benchmarks to back it up. There are lots of examples of this #58305 (no issue), #56630 (issue).

What do the performance improvements look like in your change?

[TylerBurnett]

Thank you for the prompt reply, David. Setting up benchmarks for the handlers has been tricky, but I've nearly got it down. Hopefully I can return with numbers in the coming days.

[TylerBurnett]

It's nothing to write home about, but it is measurable

Benchmark run of d12915f

Method	Mean	Error	StdDev	Median	Op/s	Gen 0	Gen 1	Gen 2	Allocated
AuthenticateAsync_ValidToken	70.24 us	2.187 us	6.311 us	69.15 us	14,236.6	-	-	-	10 KB
ChallengeAsync_ValidToken	72.62 us	1.906 us	5.499 us	72.10 us	13,770.4	-	-	-	12 KB
AuthenticateAsync_InvalidToken	61.50 us	2.374 us	6.773 us	61.20 us	16,261.3	-	-	-	11 KB
ChallengeAsync_InvalidToken	64.71 us	2.742 us	7.997 us	62.25 us	15,453.3	-	-	-	13 KB

Benchmark run of HEAD

Method	Mean	Error	StdDev	Median	Op/s	Gen 0	Gen 1	Gen 2	Allocated
AuthenticateAsync_ValidToken	68.49 us	2.326 us	6.711 us	68.00 us	14,601.6	-	-	-	10 KB
ChallengeAsync_ValidToken	74.62 us	2.617 us	7.635 us	73.65 us	13,401.2	-	-	-	11 KB
AuthenticateAsync_InvalidToken	60.42 us	2.456 us	7.203 us	59.20 us	16,550.8	-	-	-	12 KB
ChallengeAsync_InvalidToken	62.79 us	2.268 us	6.543 us	60.90 us	15,925.7	-	-	-	12 KB

Not sure what happened in the ChallengeAsync_ValidToken flow for Head

I see your point in all this regardless, If I can create measurable change and it doesn't affect code maintainability there is a possibility of it being accepted.

Thank you for your time David

[martincostello]

I notice from the commit that you enumerate validationResult.MemberNames twice. That might explain some unexpected differences.

[TylerBurnett]

Inside SetupTokenValidationParametersAsync? Thank you, thats a mistep!