Skip to content

Add micro-benchmark validation#29695

Merged
BrennanConroy merged 13 commits intomainfrom
brecon/microbench
Mar 12, 2021
Merged

Add micro-benchmark validation#29695
BrennanConroy merged 13 commits intomainfrom
brecon/microbench

Conversation

@BrennanConroy
Copy link
Member

@BrennanConroy BrennanConroy commented Jan 27, 2021 •
edited
Loading

Fixes #27607
Fixes #23025

Open question:
Should projects have to opt-in for validation? If not, we need to align on a folder/project name structure so we don't try to validate non-micro benchmark projects.

This is currently turned off because BenchmarkDotNet hasn't released a package that works with net6.0 yet. Using nightlies I can verify this locally.

On failure:

  Running benchmark validation for:
  Microsoft.AspNetCore.SignalR.Microbenchmarks

  'HubConnectionReceiveBenchmark.ReceiveAsync: Dry(Toolchain=InProcessNoEmitToolchain, IterationCount=1, LaunchCount=1, RunStrategy=ColdStart, UnrollFactor=1, WarmupCount=1) [ArgumentCount=0, MessageCount=1, Protocol=json], 0 runs' failed, reason: 'AllMeasurements'
E:\ASPNETCORE\aspnetcore\eng\targets\CSharp.Common.targets(103,5): error MSB3073: The command "dotnet E:\ASPNETCORE\aspnetcore\artifacts\bin\Microsoft.AspNetCore.SignalR.Microbenchmarks\Debug\net6.0\Microsoft.AspNetCore.SignalR.Microbenchmarks.dll -f * --validate-fast" exited with code 1. [E:\ASPNETCORE\aspnetcore\src\SignalR\perf\Microbenchmarks\Microsoft.AspNetCore.SignalR.Microbenchmarks.csproj]

Build FAILED.

@BrennanConroy BrennanConroy requested review from a team and halter73 January 27, 2021 19:08
@halter73
Copy link
Member

halter73 commented Jan 27, 2021

I'd rather fix the folder structure than make validation opt-in. BenchmarkDotNet hasn't release a package in a while now. Can we use the nightly package feed for our microbenchmarks?

@BrennanConroy
Copy link
Member Author

BrennanConroy commented Jan 27, 2021

BenchmarkDotNet hasn't release a package in a while now.

Yeah, I asked and they should be releasing an update in a week or two

@Pilchie Pilchie added the area-signalr Includes: SignalR clients and servers label Jan 27, 2021
@BrennanConroy BrennanConroy added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework and removed area-signalr Includes: SignalR clients and servers labels Jan 28, 2021
@BrennanConroy
Copy link
Member Author

BrennanConroy commented Jan 28, 2021

I like the current format for SignalR and Middleware:
C:\aspnetcore\src\SignalR\perf\Microbenchmarks\Microsoft.AspNetCore.SignalR.Microbenchmarks.csproj
C:\aspnetcore\src\Middleware\perf\Microbenchmarks\Microsoft.AspNetCore.WebSockets.Microbenchmarks.csproj

Which would make Kestrel:
C:\aspnetcore\src\Servers\Kestrel\perf\Kestrel.Performance\Microsoft.AspNetCore.Server.Kestrel.Performance.csproj -> C:\aspnetcore\src\Servers\Kestrel\perf\Microbenchmarks\Microsoft.AspNetCore.Server.Kestrel.Microbenchmarks.csproj

@halter73
Copy link
Member

halter73 commented Jan 29, 2021

Sounds good to me.

@sebastienros We're planning to move and rename the Microsoft.AspNetCore.Server.Kestrel.Performance microbenchmarks. This is just a heads up in case you need to react.

@sebastienros
Copy link
Member

sebastienros commented Feb 19, 2021

I will update our config files when it's merged, and maybe get them running in the perf CI.

@BrennanConroy BrennanConroy marked this pull request as ready for review March 10, 2021 02:54
dougbu
dougbu reviewed Mar 10, 2021
View reviewed changes
@BrennanConroy
Copy link
Member Author

BrennanConroy commented Mar 11, 2021

Any :shipit: 's 😃

javiercn
javiercn approved these changes Mar 11, 2021
View reviewed changes
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Nice!

@BrennanConroy BrennanConroy merged commit 340d161 into main Mar 12, 2021
@BrennanConroy BrennanConroy deleted the brecon/microbench branch March 12, 2021 05:28
@BrennanConroy BrennanConroy added this to the 6.0-preview3 milestone Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@javiercn javiercn javiercn approved these changes

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@pranavkm pranavkm Awaiting requested review from pranavkm

@Tratcher Tratcher Awaiting requested review from Tratcher

+2 more reviewers

@dougbu dougbu dougbu left review comments

@jkotalik jkotalik jkotalik approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Milestone

6.0-preview3

Development

Successfully merging this pull request may close these issues.

Ensure our microbenchmarks are healthy and stay healthy across the repo Update BenchmarkDotNet and remove net5.0 workaround

7 participants

@BrennanConroy @halter73 @sebastienros @dougbu @javiercn @jkotalik @Pilchie