-
Notifications
You must be signed in to change notification settings - Fork 280
Add TAR async benchmarks #2533
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
Add TAR async benchmarks #2533
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a reasonable addition, LGTM. @adamsitnik could you take a look too?
src/benchmarks/micro/libraries/System.Formats.Tar/Perf.TarFile.cs
Outdated
Show resolved
Hide resolved
src/benchmarks/micro/libraries/System.Formats.Tar/Perf.TarFile.cs
Outdated
Show resolved
Hide resolved
src/benchmarks/micro/libraries/System.Formats.Tar/Perf.TarFile.cs
Outdated
Show resolved
Hide resolved
AOT tests failing @LoopedBard3
|
@danmoseley interesting to see that nothing else failed. So does that mean microbenchmarks is already consuming preview6? @LoopedBard3 I see you merged this PR, which contains the only mention of preview6: #2399 I see globals.json mentions preview5 as dependency for "tools" (not sure what that means): Line 3 in 062e33e
Can you help me confirm what's the currently consumed .NET version in microbenchmarks? |
It looks like the one that is being used to run the microbenchmarks is version 7.0.100-rc.1.22371.5, based on the logs from the ci_setup step. There is a chance that the fix for #2496 will also help here since we are not seeing the error in those runs. |
/azp run performance-ci |
Azure Pipelines successfully started running 1 pipeline(s). |
@LoopedBard3 seems still broken. |
The centos issues seem to have been solved 4 days ago: #2532. Lets try a new run to see if it solved the issues here or if any new ones show up. |
/azp run performance-ci |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @carlossanlop !
src/benchmarks/micro/libraries/System.Formats.Tar/Perf.TarFile.cs
Outdated
Show resolved
Hide resolved
@DrewScoggins these benchmarks require Preview 7 SDK. The build is green for dotnet/performance because this repo uses the latest SDK, but how about dotnet/runtime that also builds these benchmarks? Can we merge this PR without breaking dotnet/runtime perf runs? |
Ran a test run using a version of this with BDN updated: https://dev.azure.com/dnceng/internal/_build/results?buildId=1921391&view=results. It looks like we should be good since we run are running latest runtime bits during testing. |
@LoopedBard3 thanks! |
@adamsitnik @dakersnar I'm copying the existing sync benchmarks and making them async.
Important!
Do not merge yet! This change depends on moving the performance microbenchmarks project to consume preview7. These async APIs did not exist in preview6.