-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Build and package System.Runtime.CompilerServices.Unsafe for netcoreapp20 #24565
Comments
We can add a netcoreapp2.0 configuration but we also need to keep the netcoreapp configuration so we can build the entire netcoreapp vertical against live bits. |
Can you please expand on why we need to do that? It would undo the changes from dotnet/corefx#25959 |
There are two separate things:
dotnet/corefx#25959 reversed both 1 and 2. This issue is about bringing 1 back, but keeping 2 reversed. If we do not do this, we will be missing the perf fix here: https://github.com/dotnet/corefx/blob/master/src/System.Runtime.CompilerServices.Unsafe/src/System.Runtime.CompilerServices.Unsafe.il#L269 . AsRef won't be as fast as it can be in .NET Core. |
@weshaggard is this something you could help us with for the bug bounce this month? |
Add netcoreapp2.0 configuration to package to optimize a few parts of the Unsafe APIs. https://github.com/dotnet/corefx/issues/26145 Add netcoreapp configuration so that netcoreapp vertical still builds without needing the netcoreapp2.0 targeting pack.
Add netcoreapp2.0 configuration to package to optimize a few parts of the Unsafe APIs. https://github.com/dotnet/corefx/issues/26145 Add netcoreapp configuration so that netcoreapp vertical still builds without needing the netcoreapp2.0 targeting pack.
We would have a performance regression in some of the Unsafe.As methods otherwise. It should build and packaged for netcoreapp2.0 that will cover the live netcoreapp as well.
See https://github.com/dotnet/corefx/pull/25959/files/0c0e5136caec611a7178470ff8fbcaa754ddc1a8#discussion_r159258597 for full discussion.
The text was updated successfully, but these errors were encountered: