-
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
SqlServer.Types regressed on .NET 7 #75455
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Could you try running the benchmark with |
Using .NET 7.0: The result is identical. |
could be related to WriteXorExecute. |
Sounds right, here are my results of disabling/enabling W^X.
|
@janvorli has made a fix in RC2 which might help with this. |
Installing
|
I have run it with internal W^X logging compiled in. The thing that slows it down is the number of delegates for reverse pinvoke (used for calling managed code from native code) created. During the test, 410000 such delegates for were created. That is really huge. Each such delegate created can trigger a map and unmap of writeable memory. The RTM reduces the number of mappings by caching them in a simple way, but the effectivity of the caching depends on the pattern of code generation / delegates creation. The best way to get rid of the perf regression is to stop using delegates for calling managed methods back from native code and use "UnmanagedCallersOnly" attribute marked functions or stop creating that many delegates. I guess that somewhere a new delegate is created each time the code needs to pass a managed callback to native code. Creating it once and reusing it would lower the performance hit significantly. Both approaches would be beneficial anyways as they would reduce the amount of GC garbage generated too. I guess this is in the |
Hi @jnyrup, assume you have a workaround of disabling |
@David-Engel may have input on this. |
As W^X is only strictly necessary when running on Apple Silicon (as far as I can read about W^X) and Thanks to you all for the help of finding the culprit of this performance/security clash. |
@jnyrup right, Apple Silicon has W^X always enabled. FYI, it uses a completely different mechanism (built in the CPU) to achieve that than Windows / Linux - there is no double mapping and thus no perf hit caused by that. |
@jnyrup System.Data.SqlClient is in maintenance mode and won't be receiving anything other then security/critical updates. I suggest migrating to Microsoft.Data.SqlClient where we can make changes for perf improvements, if needed. Regards, |
@janvorli Thanks for the valuable insights 👍 @David-Engel I don't think For now I've sent the package owners a message through the nuget.org about this thread. |
Just checking if this is something we need to keep open? |
Closing since this is not actionable on the runtime side. |
Description
When trying out .NET 7 Preview 7 the runtime of some our tests approximately increased by ~100% from 0.9s from 1.8s compared to .NET 6.0.8.
I traced the majority of the regression to calls to Microsoft.SqlServer.Types and bisected the regression to be introduced in .NET 7 Preview 5.
To reproduce download STIntersection.zip and run
Configuration
See each detail section below for the version of .NET used in the different benchmarks.
Regression?
Yes.
.NET 7 Preview 4 has same performance as .NET 6.0.8.
.NET 7 Preview 5 is 50% slower than .NET 6.0.8
Data
.NET 7 Preview 4
.NET 7 Preview 5
.NET 7 Preview 7
.NET 7 RC 1
In this stack trace I notice that the module of
dynamicClass.IL\_STUB\_PInvoke
issystem.net.sockets.il
🤨Partial call stack from VS
Analysis
The text was updated successfully, but these errors were encountered: