Tagging subscribers to this area: @dotnet/area-system-linq
See info inarea-owners.md if you want to be subscribed.

Oh nice:

Size statistics

Pull request#111743

I would love it if we could remove the compilation constant, have a single build, and just achieve desired size reduction in certain environments via the feature switch.

I would love it if we could remove the compilation constant, have a single build, and just achieve desired size reduction in certain environments via the feature switch.

Do you have any opinion on how to achieve the "The only problem is maintainability - how do we remember that we should only call the virtuals under a !IsSizeOptimized check." part? I was thinking about leaving the .SpeedOpt.cs files around to keep the virtuals separated at least in files. It won't help much, but it's at least something.

Do you have any opinion on how to achieve the "The only problem is maintainability - how do we remember that we should only call the virtuals under a !IsSizeOptimized check."

Hmm, I guess this is the spot where we could use the generalized feature guards.@sbomer - did we implement that part? (I'm not able to find it right now, so my guess is no, but wanted to check.)

@sbomer - did we implement that part? (I'm not able to find it right now, so my guess is no, but wanted to check.)

Nope, only for RequiresUnreferencedCode/RequiresDynamicCode/RequiresAssemblyFiles.

/azp run runtime-nativeaot-outerloop

I marked this ready for review. Not all tests pass. It turns out we cannot fully replace this with a feature switch.

This part is the problem:

We cannot express "this class only implements the interface if a feature switch is defined". And this is a popular interface.

I'll try to measure how much that matters in a separate PR (#111794). The extra interface will only really matter for AOT.

We can adjusts the test to expect this to pass, that's the least problem.

@ivanpovazan is it easy enough to measure the size impact of this on iOS? This is replacing the special build of System.Linq with a common assembly that is same across platforms. There's now a feature switch to enable size savings.

To enable the feature switch, one currently needs to add:

  <ItemGroup>    <RuntimeHostConfigurationOptionInclude="System.Linq.Enumerable.IsSizeOptimized"Value="true"Trim="true" />  </ItemGroup>

The expectation is that we would introduce a property to wrap it, and default the property on iOS/Android/etc. totrue.

Do you have any opinion on how to achieve the "The only problem is maintainability - how do we remember that we should only call the virtuals under a !IsSizeOptimized check." part? I was thinking about leaving the .SpeedOpt.cs files around to keep the virtuals separated at least in files. It won't help much, but it's at least something.

We could have an analyzer just for that library. Better would be to build out the unimplemented parts of feature guards and then use it here. We could explore having a separate build of the library that we test but don't ship and that implements the guarded functionality to throw. But probably most simply, we could rely on size regression tests to point out any place we make a mistake and then fix it, as we do with other performance-focused issues.

@ivanpovazan is it easy enough to measure the size impact of this on iOS? This is replacing the special build of System.Linq with a common assembly that is same across platforms. There's now a feature switch to enable size savings.

Unfortunately it is tricky. Especially since we would like to measure the size impact on MAUI iOS apps and net10 builds are currently unstable.

Would it be possible to apply these changes on top of net9 branch (not sure if there are any dependencies from the point we branched to net10) ? I could then use the official net9 workloads with local build of your changes to see the impact.

Would it be possible to apply these changes on top of net9 branch (not sure if there are any dependencies from the point we branched to net10) ? I could then use the official net9 workloads with local build of your changes to see the impact.

Yeah, this applies pretty cleanly to 9.0 and I don't think there's prerequisites. Here's a branch:https://github.com/MichalStrehovsky/runtime/pull/new/sizeoptswitch-net9

The important bit when measuring is that we need to set the feature switch, or this will be a regression for sure.

Here are the numbers for the couple test apps with native AOT using OPTIMIZE_FOR_SIZE. It is better with the ifdef, but not by much. I'd expect a similar picture for Mono AOT.

Size statistics

Pull request#111794

Here are the numbers for the couple test apps with native AOT using OPTIMIZE_FOR_SIZE. It is better with the ifdef, but not by much.

(Need to compare with the table in my previous comment, not the "size before". The "size before" is main branch.)

But probably most simply, we could rely on size regression tests to point out any place we make a mistake and then fix it, as we do with other performance-focused issues.

Yeah, I guess that would work. It will require some skill to root cause but if enough people remember that some virtuals are special (and we should probably keep them special in the .SpeedOpt.cs files), it should be workable.

/azp run runtime-nativeaot-outerloop

@MichalStrehovsky It seems the optimization only cuts-off the regression introduced, is that expected?

Yep, this just replaces ifdef with a configurable feature switch. There were valid questions in#109978 (that introduces a feature switch for LINQ) why we should have both ifdefs and feature switches. So this is a preparatory change for that.

Related pull requests:

• Set default for UseSizeOptimizedLinq macios#22074
• Set default for UseSizeOptimizedLinq android#9718
• AddUseSizeOptimizedLinq feature switch sdk#46375

/azp run runtime-nativeaot-outerloop

/azp run runtime-extra-platforms