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

Enable endpoint routing in aspnetcore benchmark #1418

Merged
merged 1 commit into from
Apr 21, 2021

Conversation

kevingosse
Copy link
Collaborator

The impact of changes introduced in #1289 can only be measured with endpoint routing enabled.

It also means upgrading the benchmark from aspnetcore 2.x to 3.x, but it's probably more representative of what customers use.

@kevingosse kevingosse requested a review from a team as a code owner April 21, 2021 14:10
@kevingosse kevingosse added the area:benchmarks Benchmarks, throughput tests, Crank, Bombardier, etc label Apr 21, 2021
Copy link
Member

@andrewlock andrewlock left a comment

Choose a reason for hiding this comment

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

Is it worth benchmarking both with and without endpoint routing? You could create a separate Startup class for that and continue to use UseMvc()?

@kevingosse
Copy link
Collaborator Author

Is it worth benchmarking both with and without endpoint routing? You could create a separate Startup class for that and continue to use UseMvc()?

I considered it but I don't think we have any code path that is triggered only when endpoint routing is disabled. And the benchmark pipeline is already taking a while so I'm cautious about adding new benchmarks 😃

@kevingosse kevingosse merged commit c809d7d into master Apr 21, 2021
@kevingosse kevingosse deleted the kevin/benchmark_routing branch April 21, 2021 16:41
@andrewlock andrewlock added this to the 1.26.1 milestone Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:benchmarks Benchmarks, throughput tests, Crank, Bombardier, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants