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

Cannot benchmark netcoreapp3.0 #2333

Closed
timcassell opened this issue Jun 19, 2023 · 16 comments · Fixed by #2352 or #2359
Closed

Cannot benchmark netcoreapp3.0 #2333

timcassell opened this issue Jun 19, 2023 · 16 comments · Fixed by #2352 or #2359
Milestone

Comments

@timcassell
Copy link
Collaborator

I got this error when trying to run a benchmark on CoreRuntime.Core30. Should BDN try to fix this and keep supporting older build targets, or just drop support?

// Build Error: Standard output:

 Standard error:
 MSBuild version 17.7.0-preview-23281-03+4ce2ff1f8 for .NET
C:\Users\Tim\.nuget\packages\system.runtime.compilerservices.unsafe\6.0.0\buildTransitive\netcoreapp2.0\System.Runtime.CompilerServices.Unsafe.targets(4,5): error : System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp3.0. Consider updating your TargetFramework to netcoreapp3.1 or later. [C:\Users\Tim\Documents\git\ProtoPromiseBenchmarks\AsynchronousBenchmarks\bin\Release\net472\c449f68f-17cc-4372-a6f3-e6f7e2ae26c3\BenchmarkDotNet.Autogenerated.csproj]
Build FAILED.
C:\Users\Tim\.nuget\packages\system.runtime.compilerservices.unsafe\6.0.0\buildTransitive\netcoreapp2.0\System.Runtime.CompilerServices.Unsafe.targets(4,5): error : System.Runtime.CompilerServices.Unsafe doesn't support netcoreapp3.0. Consider updating your TargetFramework to netcoreapp3.1 or later. [C:\Users\Tim\Documents\git\ProtoPromiseBenchmarks\AsynchronousBenchmarks\bin\Release\net472\c449f68f-17cc-4372-a6f3-e6f7e2ae26c3\BenchmarkDotNet.Autogenerated.csproj]
    0 Warning(s)
    1 Error(s)
Time Elapsed 00:00:00.64
@adamsitnik
Copy link
Member

Hi @timcassell !

This error comes from the SDK, as far as I know there is nothing we can do about it on our side.

Moreover, it seems to be related to netcoreapp2.0 version of the Unsafe package?

netcoreapp2.0\System.Runtime.CompilerServices.Unsafe

Which is not supported anymore, so I don't expect anyone to fix it on the SDK side.

@adamsitnik adamsitnik closed this as not planned Won't fix, can't repro, duplicate, stale Jun 20, 2023
@timcassell
Copy link
Collaborator Author

timcassell commented Jun 20, 2023

@adamsitnik If we really can't do anything about it, then core versions older than 3.1 should be removed. Otherwise, to make it work again, I think adding a netcoreapp2.0 BDN build target that uses older nuget package versions should do the trick.

cc @AndreyAkinshin

[Edit] Also the latest preview version of the Unsafe package dropped 3.1 support, so even 3.1 and 5.0 may stop working if it's updated.

@AndreyAkinshin
Copy link
Member

@timcassell I think we can safely drop support of all .NET Core versions below 3.1 ("End of support" for 3.0 happened more than two years ago). Would you like to send a PR with proper fixes?

@timcassell
Copy link
Collaborator Author

@AndreyAkinshin Should we drop support for any core version below 6.0? Or only below 3.1?

@AndreyAkinshin
Copy link
Member

Only below 3.1. Typically, we try to support all the old runtimes while it doesn't require additional maintenance efforts.

@adamsitnik
Copy link
Member

If we really can't do anything about it, then core versions older than 3.1 should be removed.

But people can still use 3.1 SDK and it should work just fine.

My point is that if a newer SDK does not allow the users to build apps targeting older TFMs we can't do anything about it. But we should also not change the code, as people might still use older SDKs and everything should work fine.

@timcassell
Copy link
Collaborator Author

Using an older SDK doesn't make it work. It's a problem with the nuget package. I tried with sdk 3.0.103 and got the same error.

@adamsitnik
Copy link
Member

@ViktorHofer does it mean that using newer version of packages like System.Runtime.CompilerServices.Unsafe that still target netstandard2.0 will actually not work for old tfm like netcoreapp3.1 etc?

@adamsitnik adamsitnik reopened this Jul 4, 2023
@ViktorHofer
Copy link
Member

Yes that behavior was introduced with .NET 6 packages: dotnet/announcements#190

  • For .NET 7 packages the supported TFMs are net6.0 and net7.0
  • For .NET 8 packages the supported TFMs are net6.0, net7.0 and net8.0
  • Similarly for .NET 9 packages, the supported TFMs will be net8.0 and net9.0

Note that the .NET 6 packages emitted an error when being consumed on an unsupported TFM. We then later changed that to a warning.

@timcassell
Copy link
Collaborator Author

How does that explain the issue? Netcoreapp3.0 implements netstandard2.0, which the net6 packages still support, so why does it not just consume the netstandard2.0 dlls rather than error?

@ViktorHofer
Copy link
Member

In the linked announcement it states that a .NET 6 package supports netcoreapp3.1, net5.0 and net6.0 but not netcoreapp3.0. Overall, netcoreapp3.0, netcoreapp3.1 and net5.0 are all out-of-support and shouldn't be used anymore.

@timcassell
Copy link
Collaborator Author

It also says that it supports netstandard2.0.

@ViktorHofer
Copy link
Member

While the package supports netstandard2.0, it explicitly lists the .NETCoreApp TFMs that are supported: netcoreapp3.1, net5.0 and net6.0.

Please see open-telemetry/opentelemetry-dotnet#3448 which has much more information on that subject.

@timcassell
Copy link
Collaborator Author

timcassell commented Jul 5, 2023

Okay, from reading that over with more understanding of why netstandard2.0 is not actually supported for netcoreapp3.0, it seems we have 3 options here.

  1. Drop netcoreapp3.0 and earlier support in BDN
  2. Conditionally use net5 nuget packages for netstandard2.0 target and net6 packages for net6.0 target
  3. Add PrivateAssets=buildTransitive to the problem packages.

3 seems to be an unsafe option, as not everything is guaranteed to work. So really only 2 options.

@AndreyAkinshin What do you think?

[Edit] Or option 4 that I mentioned earlier, add a netcoreapp2.0 BDN build target that uses older nuget package versions

@ViktorHofer
Copy link
Member

Exactly, you don't want to go with option 3, as you are in untested territory. Regarding option 1 vs 2, I would recommend to drop out-of-support TFMs from the current release. If someone really must stay on an unsupported runtime (for whatever reason), they can do so an also stay on a previous released version of BenchmarkDotNet.

The current (since the .NET 7 release) minimum supported frameworks are:

  • net462
  • net6.0
  • netstandard2.0

Just curious, why does BDN target .NET Standard? I would expect benchmark apps to target a specific runtime like .NET Framework or .NETCoreApp.

@adamsitnik
Copy link
Member

While the package supports netstandard2.0, it explicitly lists the .NETCoreApp TFMs that are supported: netcoreapp3.1, net5.0 and net6.0.

I find it confusing, I would expect it to simply support only netcoreapp3.1, net5.0 and net6.0 in explicit way.

Just curious, why does BDN target .NET Standard?

We support all runtimes and architectures and wanted to reduce the number of #if defs and targeted TFMs.

Conditionally use net5 nuget packages for netstandard2.0 target and net6 packages for net6.0 target

I believe this is the way to go.

If someone really must stay on an unsupported runtime (for whatever reason), they can do so an also stay on a previous released version of BenchmarkDotNet.

That is true, but on the other hand once in a while I need to find a version of .NET Core where perf change was introduced and I benchmark all of them. Once I find the version I user the profiler and disassembler plugins to understand what exactly has changed. These plugins are improving over time and work fine with older runtimes.

Others often benchmark current vs old .NET version to show how it has changed over time. If they use an old BDN version it may not support current .NET version.

So we are definitely going to keep .NET Standard 2.0 support for years to come. And live with the pain it causes to us ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants