Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 25, 2025

Removes <Optimize>True</Optimize> from all .csproj files under src/tests/async, along with any <PropertyGroup> elements left empty after removal.

Changes

  • 31 files: Removed entire <PropertyGroup> block (contained only <Optimize>True</Optimize>)
  • 11 files: Removed <Optimize>True</Optimize> line only, preserving other properties (CLRTestKind, RequiresProcessIsolation, DefineConstants, etc.)

Before:

<Project Sdk="Microsoft.NET.Sdk.IL">
  <PropertyGroup>
    <Optimize>True</Optimize>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="$(MSBuildProjectName).cs" />
  </ItemGroup>
</Project>

After:

<Project Sdk="Microsoft.NET.Sdk.IL">
  <ItemGroup>
    <Compile Include="$(MSBuildProjectName).cs" />
  </ItemGroup>
</Project>
Original prompt

remove <Optimize>True</Optimize> from all project files under src/tests/async. Also remove any empty <PropertyGroup> that are left in the project file after removing the Optimize tag


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Remove <Optimize>True</Optimize> from project files and clean up Remove <Optimize>True</Optimize> from src/tests/async project files Nov 25, 2025
Copilot finished work on behalf of MichalStrehovsky November 25, 2025 06:34
@MichalStrehovsky MichalStrehovsky marked this pull request as ready for review November 25, 2025 06:41
Copilot AI review requested due to automatic review settings November 25, 2025 06:41
@MichalStrehovsky
Copy link
Member

/azp run runtime-nativeaot-outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

Copilot AI left a 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.

@jakobbotsch
Copy link
Member

jakobbotsch commented Nov 25, 2025

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.

@MichalStrehovsky
Copy link
Member

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.)

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)

What is the NativeAOT behavior that is being tested here when the assemblies are unoptimized?

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.

Copy link
Member

@jakobbotsch jakobbotsch left a 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.

@MichalStrehovsky
Copy link
Member

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
Debug run of the async tree: Time Elapsed 00:00:37.54

@MichalStrehovsky
Copy link
Member

/ba-g timeouts in legs that are not related

@MichalStrehovsky MichalStrehovsky merged commit 3266d23 into main Nov 26, 2025
102 of 105 checks passed
@MichalStrehovsky MichalStrehovsky deleted the copilot/remove-optimize-tag-and-empty-property-groups branch November 26, 2025 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants