-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Remove <Optimize>True</Optimize> from src/tests/async project files
#121945
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
Remove <Optimize>True</Optimize> from src/tests/async project files
#121945
Conversation
Co-authored-by: MichalStrehovsky <[email protected]>
<Optimize>True</Optimize> from src/tests/async project files
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes redundant <Optimize>True</Optimize> settings from 42 async test project files, relying instead on the default optimization configuration already defined in the parent Directory.Build.props which sets optimization based on the build configuration (Debug, Release, or Checked).
- Removes explicit optimization overrides that duplicated parent configuration
- Cleans up empty
<PropertyGroup>elements left after removal - Standardizes project file structure across async tests
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| without-yields/without-yields.csproj | Removed entire PropertyGroup containing only Optimize setting |
| with-yields/with-yields.csproj | Removed entire PropertyGroup containing only Optimize setting |
| widening-tailcall/widening-tailcall.csproj | Removed entire PropertyGroup containing only Optimize setting |
| void/void.csproj | Removed entire PropertyGroup containing only Optimize setting |
| varying-yields/varying-yields.csproj | Removed entire PropertyGroup containing only Optimize setting |
| varying-yields/varying-yields-valuetask.csproj | Removed Optimize setting, preserved DefineConstants |
| varying-yields/varying-yields-task.csproj | Removed Optimize setting, preserved DefineConstants |
| valuetaskbased-asyncfibonacci-without-yields/valuetaskbased-asyncfibonacci-without-yields.csproj | Removed Optimize setting, preserved CLRTestKind |
| valuetaskbased-asyncfibonacci-with-yields/valuetaskbased-asyncfibonacci-with-yields.csproj | Removed Optimize setting, preserved CLRTestKind |
| valuetask/valuetask.csproj | Removed entire PropertyGroup containing only Optimize setting |
| valuetask-source/valuetask-source.csproj | Removed Optimize setting, preserved RequiresProcessIsolation |
| taskbased-asyncfibonacci-without-yields/taskbased-asyncfibonacci-without-yields.csproj | Removed Optimize setting, preserved CLRTestKind |
| taskbased-asyncfibonacci-with-yields/taskbased-asyncfibonacci-with-yields.csproj | Removed Optimize setting, preserved CLRTestKind |
| synchronization-context/synchronization-context.csproj | Removed entire PropertyGroup containing only Optimize setting |
| syncfibonacci-without-yields/syncfibonacci-without-yields.csproj | Removed Optimize setting, preserved CLRTestKind |
| struct/struct.csproj | Removed entire PropertyGroup containing only Optimize setting |
| strength-reduction/strength-reduction.csproj | Removed entire PropertyGroup containing only Optimize setting |
| small/small.csproj | Removed entire PropertyGroup containing only Optimize setting |
| simple-eh/simple-eh.csproj | Removed entire PropertyGroup containing only Optimize setting |
| shared-generic/shared-generic.csproj | Removed entire PropertyGroup containing only Optimize setting |
| returns/returns.csproj | Removed entire PropertyGroup containing only Optimize setting |
| reflection/reflection-simple.csproj | Removed entire PropertyGroup containing only Optimize setting |
| pinvoke/pinvoke.csproj | Removed entire PropertyGroup containing only Optimize setting |
| pgo/pgo.csproj | Removed entire PropertyGroup containing only Optimize setting |
| override/override.csproj | Removed entire PropertyGroup containing only Optimize setting |
| objects-captured/objects-captured.csproj | Removed entire PropertyGroup containing only Optimize setting |
| object/object.csproj | Removed entire PropertyGroup containing only Optimize setting |
| mincallcost-microbench/mincallcost-microbench.csproj | Removed Optimize setting, preserved CLRTestKind |
| inst-unbox-thunks/inst-unbox-thunks.csproj | Removed entire PropertyGroup containing only Optimize setting |
| implement/implement.csproj | Removed entire PropertyGroup containing only Optimize setting |
| gc-roots-scan/gc-roots-scan.csproj | Removed Optimize setting, preserved CLRTestKind |
| fibonacci-without-yields/fibonacci-without-yields.csproj | Removed entire PropertyGroup containing only Optimize setting |
| fibonacci-without-yields-config-await/fibonacci-without-yields-config-await.csproj | Removed entire PropertyGroup containing only Optimize setting |
| fibonacci-with-yields/fibonacci-with-yields.csproj | Removed entire PropertyGroup containing only Optimize setting |
| fibonacci-with-yields-struct-return/fibonacci-with-yields-struct-return.csproj | Removed entire PropertyGroup containing only Optimize setting |
| execution-context/execution-context.csproj | Removed entire PropertyGroup containing only Optimize setting |
| eh-microbench/eh-microbench.csproj | Removed Optimize setting, preserved CLRTestKind |
| cse-array-index-byref/cse-array-index-byref.csproj | Removed entire PropertyGroup containing only Optimize setting |
| collectible-alc/collectible-alc.csproj | Removed Optimize setting, preserved RequiresProcessIsolation |
| cancellation/cancellation.csproj | Removed entire PropertyGroup containing only Optimize setting |
| byref-param/byref-param.csproj | Removed entire PropertyGroup containing only Optimize setting |
| awaitingnotasync/awaitingnotasync.csproj | Removed entire PropertyGroup containing only Optimize setting |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
Does this change anything for coreclr testing? Do we now compile these unoptimized in any PR runs? (It would be unfortunate to do that – many of these tests are quite long running in debug.) What is the NativeAOT behavior that is being tested here when the assemblies are unoptimized? Note that we end up testing both the unoptimized and optimized JIT compilations of these tests because of tiered compilation. If the assemblies are compiled unoptimized they will only test unoptimized codegen. |
CoreCLR testing uses the IL that is the result of the "Build coreclr Common Pri0 Test Build AnyOS AnyCPU checked" leg. We only build the test tree once and the configuration is Checked (i.e. optimized, DEBUG defined). So this should have no effect on CoreCLR testing (I think we have a test gap that we never test without C# compiler optimizations lots of tests in the src/tests tree since only the JIT subtree does the _r.cspro/_ro.csproj/_d.csproj split)
Native AOT compilation picks up the value for optimization setting from the entrypoint assembly Optimization flag. When the entrypoint assembly Optimization flag is set to true, we compile with codegen optimizations and whole program optimizations. E.g. a test testing virtual method calls would have all of the virtual method calls devirtualized and possibly even inlined, not actually testing virtual call behaviors (in the spirit). Native AOT testing builds the test tree in both debug and release to get the full coverage. We don't have Tier-0 to simulate unoptimized. If test forces itself to be optimized, we never test/execute unoptimized code for it. |
jakobbotsch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable given the above explanation then.
You might want to double check if the time it takes to run the tests in debug is acceptable. @davidwrighton identified the slow tests in #121862 and we should perhaps just decrease their number of iterations always.
Checked run of the async tree: Time Elapsed 00:00:33.87 |
|
/ba-g timeouts in legs that are not related |
Removes
<Optimize>True</Optimize>from all.csprojfiles undersrc/tests/async, along with any<PropertyGroup>elements left empty after removal.Changes
<PropertyGroup>block (contained only<Optimize>True</Optimize>)<Optimize>True</Optimize>line only, preserving other properties (CLRTestKind,RequiresProcessIsolation,DefineConstants, etc.)Before:
After:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.