BenchmarkDotNet icon indicating copy to clipboard operation
BenchmarkDotNet copied to clipboard

Add .NET 10 support

Open am11 opened this issue 1 year ago • 2 comments

This strongly-typed enums based implementation is not scaling very well; but for now, lets add 10.0 and soon after lets see if we can make it less verbose to ease up future updates a bit: https://github.com/dotnet/runtime/pull/106599#issuecomment-2365265969

cc @adamsitnik, @lewing

am11 avatar Sep 22 '24 15:09 am11

I've tested this PR against a local build from https://github.com/dotnet/runtime/pull/106599 and it fails because it seems that the .NET 10 SDK doesn't actually allow to target .NET 10, rather it only allows targetting .NET 9. To get things to work we do need to make changes to the SDK validator to allow backwards compatibility. However, this change will still be useful to have in once we do have .NET 10 targeting supported

caaavik-msft avatar Sep 23 '24 12:09 caaavik-msft

Yup, it's a chicken-egg situation atm, and we will soon be on alpha1 in runtime repo (based on how it was done last year, around this time). I think we can pass --corerun option in local builds that points to built corerun under artifacts/bin or artifacts/tests/coreclr/<platform>/Tests/Core_Root to get the ball rolling?

am11 avatar Sep 23 '24 12:09 am11

IMHO the best direction would be to introduce something similar to the [SupportedOSPlatform], such as [SupportedFramework("netX.0")], which would provide indefinite scalability. The current hardcoded enum approach doesn't scale well, which was precisely why [SupportedPlatform("")] (introduced in .NET 5.0) was designed to be string-based instead of enum-based—managing an enum over the years would have been a maintenance nightmare.

This transition to .NET 10.0 might be an ideal time to consider such a change and start deprecating the old enum, with the goal of fully removing it after a few release cycles. Additionally, pseudo-TFMs like nativeaot9.0 feel somewhat unnatural compared to the broader ecosystem’s conventions, such as using net9.0 combined with --aot (e.g., dotnet new console -f net9.0 --aot) or simply setting PublishAot=true.

am11 avatar Oct 20 '24 23:10 am11

I don't understand how [SupportedFramework("netX.0")] would be used. I think you should open an issue with more details rather than discuss it here. And/or create a WIP PR showcasing a new system.

timcassell avatar Oct 20 '24 23:10 timcassell

I think now may be a good time to revisit this PR. Is there anything specifically blocking it or just another pass to clean up the suggested changes? We are hitting an issue for our performance wasm testing due to wasmnet100/wasmnet10_0. FYI @caaavik-msft

LoopedBard3 avatar Jan 02 '25 20:01 LoopedBard3

@timcassell is there anything else that needs to be done before merging this? It seems maybe the TryParse update, but wanted to check if there was anything else.

LoopedBard3 avatar Jan 03 '25 22:01 LoopedBard3

I think that's the only thing. And the test cases.

timcassell avatar Jan 03 '25 22:01 timcassell

@LoopedBard3 Regarding wasm specifically, you could also send a PR to fix the wasm logic so you can use the simple Wasm moniker and not need to continually update the moniker every year. https://github.com/dotnet/BenchmarkDotNet/pull/2641#issuecomment-2365334620

timcassell avatar Jan 03 '25 23:01 timcassell

I have a PR against the base branch for this PR here: https://github.com/am11/BenchmarkDotNet/pull/1. If @am11 accepts it or makes the changes, we should merge this, otherwise I will make a new PR tomorrow and link it back to this one.

LoopedBard3 avatar Jan 07 '25 01:01 LoopedBard3