-
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
Enable W^X by default #69672
Enable W^X by default #69672
Conversation
This change enables the W^X by default. In the recent past, I've made some changes to the related runtime code to improve the startup performance with W^X enabled by reducing the number of times writeable mappings need to be created. The current startup time regression is about 5..10% (measured using ASPNet plaintext, json, fortunes and orchard benchmarks on x64 windows and linux and arm64 linux). The steady state performance is the same with and without the W^X enabled. There are still opportunities for improving the startup performance, e.g. in the dynamic helpers, but it is time to enable W^X by default so that if there are unexpected consequences that were not caught by the benchmarking, we still have time to fix them for .NET 7.
The |
I've found the culprit causing the libraries test failure and fixed it here: #69697 |
Suspect we may be seeing some perf wins from this... maybe? dotnet/perf-autofiling-issues#5562 |
Sorry, didn't see the intermediate messages, my sarcasm will get me fired someday. |
Interesting that Time-to-First-Request (aka startup) is seeing an improvement. We were thinking that it will be slightly slower. |
No, that is a regression - it means it takes more time to respond on the first request 🙂 but I assume the additional security from W^X worth it, I just hope it didn't eat all the benefits we got by enabling OSR for 7.0 😆 |
Ok, that makes sense. Sorry should have looked at the charts closely, but Andy's comment above threw me off :( |
From our analysis while creating the perf report for August, we found the following regressions that seemed to line up with this PR specifically for Ubuntu 18.04 x64:
System.Security.Cryptography.X509Certificates.Tests.X509ChainTests.BuildX509ChainContoso
System.Text.RegularExpressions.Tests.Perf_Regex_Common.CtorInvoke(Options: IgnoreCase, Compiled)
@janvorli: would you consider these regressions as "by design" as there are closed auto-filed regressions above? |
@mrsharm let me look into these so that I can provide a definitive answer. |
This change enables the W^X by default. In the recent past, I've made
some changes to the related runtime code to improve the startup
performance with W^X enabled by reducing the number of times writeable
mappings need to be created. The current startup time regression is
about 5..10% (measured using ASPNet plaintext, json, fortunes and
orchard benchmarks on x64 windows and linux and arm64 linux).
The steady state performance is the same with and without the W^X
enabled.
There are still opportunities for improving the startup performance,
e.g. in the dynamic helpers, but it is time to enable W^X by default so
that if there are unexpected consequences that were not caught by the
benchmarking, we still have time to fix them for .NET 7.