Skip to content
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

.NET 8 slower than .NET 7? #4

Closed
kcrg opened this issue Sep 15, 2023 · 9 comments
Closed

.NET 8 slower than .NET 7? #4

kcrg opened this issue Sep 15, 2023 · 9 comments

Comments

@kcrg
Copy link

kcrg commented Sep 15, 2023

.NET 8 RC1 .NET 7 Xamarin.Forms
Screenshot_20230915_050941 Screenshot_20230915_050935 Screenshot_20230915_051358

It's not a big deal, but I'm curious why 🧐

Device: Samsung Galaxy A34 5G Android 13

@jonathanpeppers
Copy link
Owner

Are these Release builds?

Considering how the .NET 7/8 are so much slower, I would assume those might be using the interpreter like a Debug build would? This is turned on for Debug to enable C# hot reload.

@jonathanpeppers
Copy link
Owner

I may have read the screenshots backwards, it's been a while. Higher is better.

@kcrg
Copy link
Author

kcrg commented Sep 15, 2023

@jonathanpeppers Both Maui apps were built with default Release settings. For .NET 8, I added a new project to quickly accommodate the changes between .NET 7 and 8 and copy-pasted the code to MainPage.

.NET 7 csproj
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
  	<TargetFrameworks>net7.0-android;net7.0-ios;net7.0-maccatalyst</TargetFrameworks>
  	<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net7.0-windows10.0.19041.0</TargetFrameworks>
  	<DisableTransitiveFrameworkReferenceDownloads>true</DisableTransitiveFrameworkReferenceDownloads>
  	<OutputType>Exe</OutputType>
  	<RootNamespace>lols</RootNamespace>
  	<UseMaui>true</UseMaui>
  	<SingleProject>true</SingleProject>
  	<ImplicitUsings>enable</ImplicitUsings>

  	<!-- Display name -->
  	<ApplicationTitle>lols</ApplicationTitle>

  	<!-- App Identifier -->
  	<ApplicationId>com.companyname.lols</ApplicationId>
  	<ApplicationIdGuid>c8f35716-0092-4ac1-8a44-299f94f7df40</ApplicationIdGuid>

  	<!-- Versions -->
  	<ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
  	<ApplicationVersion>1</ApplicationVersion>
  </PropertyGroup>

  <PropertyGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'windows'">
  	<SupportedOSPlatformVersion>10.0.17763.0</SupportedOSPlatformVersion>
  	<TargetPlatformMinVersion>10.0.17763.0</TargetPlatformMinVersion>
  </PropertyGroup>

  <PropertyGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'android'">
  	<SupportedOSPlatformVersion>21.0</SupportedOSPlatformVersion>
  	<AndroidPackageFormat>apk</AndroidPackageFormat>
  	<!-- Update if you want to run on emulator, etc. -->
  	<!--<RuntimeIdentifier>android-arm64</RuntimeIdentifier>-->
  </PropertyGroup>

  <PropertyGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'ios'">
  	<SupportedOSPlatformVersion>14.2</SupportedOSPlatformVersion>
  	<ProvisioningType>automatic</ProvisioningType>
  	<CodesignKey>Apple Development: Created via API (7DDTKCUL5G)</CodesignKey>
  	<CodesignProvision>VS: WildCard Development</CodesignProvision>
  </PropertyGroup>

  <PropertyGroup Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'maccatalyst'">
  	<SupportedOSPlatformVersion>14.0</SupportedOSPlatformVersion>
  </PropertyGroup>

  <ItemGroup>
  	<!-- App Icon -->
  	<MauiIcon Include="Resources\AppIcon\appicon.svg" ForegroundFile="Resources\AppIcon\appiconfg.svg" Color="#512BD4" />

  	<!-- Splash Screen -->
  	<MauiSplashScreen Include="Resources\Splash\splash.svg" Color="#512BD4" BaseSize="128,128" />

  	<!-- Images -->
  	<MauiImage Include="Resources\Images\*" />
  	<MauiImage Update="Resources\Images\dotnet_bot.svg" BaseSize="168,208" />

  	<!-- Custom Fonts -->
  	<MauiFont Include="Resources\Fonts\*" />

  	<!-- Raw Assets (also remove the "Resources\Raw" prefix) -->
  	<MauiAsset Include="Resources\Raw\**" LogicalName="%(RecursiveDir)%(Filename)%(Extension)" />
  </ItemGroup>

  <ItemGroup>
  	<PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="7.0.0" />
  </ItemGroup>

</Project>

.NET 8 RC1 csproj
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
  	<TargetFrameworks>net8.0-android;net8.0-ios;net8.0-maccatalyst</TargetFrameworks>
  	<TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net8.0-windows10.0.19041.0</TargetFrameworks>
  	<!-- Uncomment to also build the tizen app. You will need to install tizen by following this: https://github.com/Samsung/Tizen.NET -->
  	<!-- <TargetFrameworks>$(TargetFrameworks);net8.0-tizen</TargetFrameworks> -->

  	<!-- Note for MacCatalyst:
  	The default runtime is maccatalyst-x64, except in Release config, in which case the default is maccatalyst-x64;maccatalyst-arm64.
  	When specifying both architectures, use the plural <RuntimeIdentifiers> instead of the singular <RuntimeIdentifer>.
  	The Mac App Store will NOT accept apps with ONLY maccatalyst-arm64 indicated;
  	either BOTH runtimes must be indicated or ONLY macatalyst-x64. -->
  	<!-- ex. <RuntimeIdentifiers>maccatalyst-x64;maccatalyst-arm64</RuntimeIdentifiers> -->

  	<OutputType>Exe</OutputType>
  	<RootNamespace>lols.maui8</RootNamespace>
  	<UseMaui>true</UseMaui>
  	<SingleProject>true</SingleProject>
  	<ImplicitUsings>enable</ImplicitUsings>

  	<!-- Display name -->
  	<ApplicationTitle>lols.maui8</ApplicationTitle>

  	<!-- App Identifier -->
  	<ApplicationId>com.companyname.lols.maui8</ApplicationId>

  	<!-- Versions -->
  	<ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
  	<ApplicationVersion>1</ApplicationVersion>

  	<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'ios'">11.0</SupportedOSPlatformVersion>
  	<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'maccatalyst'">13.1</SupportedOSPlatformVersion>
  	<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'android'">21.0</SupportedOSPlatformVersion>
  	<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'windows'">10.0.17763.0</SupportedOSPlatformVersion>
  	<TargetPlatformMinVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'windows'">10.0.17763.0</TargetPlatformMinVersion>
  	<SupportedOSPlatformVersion Condition="$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)')) == 'tizen'">6.5</SupportedOSPlatformVersion>
  </PropertyGroup>

  <ItemGroup>
  	<!-- App Icon -->
  	<MauiIcon Include="Resources\AppIcon\appicon.svg" ForegroundFile="Resources\AppIcon\appiconfg.svg" Color="#512BD4" />

  	<!-- Splash Screen -->
  	<MauiSplashScreen Include="Resources\Splash\splash.svg" Color="#512BD4" BaseSize="128,128" />

  	<!-- Images -->

  	<!-- Custom Fonts -->
  	<MauiFont Include="Resources\Fonts\*" />

  	<!-- Raw Assets (also remove the "Resources\Raw" prefix) -->
  </ItemGroup>

  <ItemGroup>
  	<PackageReference Include="Microsoft.Maui.Controls" Version="$(MauiVersion)" />
  	<PackageReference Include="Microsoft.Maui.Controls.Compatibility" Version="$(MauiVersion)" />
  	<PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="8.0.0-rc.1.23419.4" />
  </ItemGroup>

  <!-- Build Properties must be defined within these property groups to ensure successful publishing
     to the Mac App Store. See: https://aka.ms/maui-publish-app-store#define-build-properties-in-your-project-file -->
<PropertyGroup Condition="$(TargetFramework.Contains('-maccatalyst')) and '$(Configuration)' == 'Debug'">
  <CodesignEntitlements>Platforms/MacCatalyst/Entitlements.Debug.plist</CodesignEntitlements>
</PropertyGroup>

<PropertyGroup Condition="$(TargetFramework.Contains('-maccatalyst')) and '$(Configuration)' == 'Release'">
  <CodesignEntitlements>Platforms/MacCatalyst/Entitlements.Release.plist</CodesignEntitlements>
  <UseHardenedRuntime>true</UseHardenedRuntime>
</PropertyGroup>
</Project>

@jonathanpeppers
Copy link
Owner

What device is this?

@kcrg
Copy link
Author

kcrg commented Sep 15, 2023

@jonathanpeppers

Device: Samsung Galaxy A34 5G Android 13

@jonathanpeppers
Copy link
Owner

I'll put this on my list to dotnet-trace both.

My first guess the regression might be: dotnet/maui#13818 (review)

But maybe I can just fix one thing and get .NET 7 & 8 to at least be the same for GA?

@jonathanpeppers
Copy link
Owner

Yeah, the "setter specificity" PR contributes about this much:

.NET 7
 8.5% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
 1.2% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue
.NET 8
11.0% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
 2.8% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue

4.1% (times 377/second) is about 15 LOLs per second, just as a guess to how many LOLs that might be.

So, there might be something else slowing down as well?

jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Sep 20, 2023
Context: dotnet#13818 (review)
Context: jonathanpeppers/lols#4
Fixes: dotnet#17520

A customer noticed my LOLs per second sample was slower in .NET 8 than
.NET 7. I could reproduce their results.

Digging in, `dotnet-trace` showed one culprit was:

    .NET 7
     8.5% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
     1.2% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue
    .NET 8
    11.0% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
     2.8% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue

I knew that dotnet#13818 had some performance impact, as I noted when
reviewing the change.

Drilling in further, most of the time is spent calling
`SortedList.Last()`. Which makes sense, as `BindableObject.GetValue()`
is called *a lot* in a typical .NET MAUI application.

Adding some logging, I found my LOLs app most commonly had the following
specificity values when `BindableProperty`'s are set:

* 5,284 - a single specificity value
* 34,306 - two specificity values

No `BindableProperty`'s in this app had more than two specificity values.

So, an improvement here would be to:

* Avoid `SortedList` for the most common calls

* Make fields that store up to two specificity values

* If a *third* specificity value is required, fall back to using
`SortedList`.

I introduced a new, internal `SetterSpecificityList` class for this logic.

The results of running `BindingBenchmarker`:

    > .\bin\dotnet\dotnet.exe run --project .\src\Core\tests\Benchmarks\Core.Benchmarks.csproj -c Release -- --filter Microsoft.Maui.Benchmarks.BindingBenchmarker.*
    ...
    Before:
    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 31.67 us | 0.689 us | 2.009 us | 1.7395 | 1.7090 |  14.45 KB |
    |        BindChild | 42.18 us | 0.864 us | 2.548 us | 2.4414 | 2.3804 |  20.16 KB |
    | BindChildIndexer | 78.37 us | 1.564 us | 3.266 us | 3.5400 | 3.4180 |  29.69 KB |
    After:
    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 27.13 us | 0.521 us | 1.016 us | 1.3733 | 1.3428 |  11.33 KB |
    |        BindChild | 37.77 us | 0.845 us | 2.437 us | 2.0752 | 2.0142 |  17.03 KB |
    | BindChildIndexer | 69.45 us | 1.356 us | 2.859 us | 3.1738 | 3.0518 |  26.56 KB |

My original numbers (before specificity changes in dotnet#13818) were:

    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 24.46 us | 0.554 us | 1.624 us | 1.2512 | 1.2207 |  10.23 KB |
    |        BindChild | 33.21 us | 0.743 us | 2.192 us | 1.9226 | 1.8921 |  15.94 KB |
    | BindChildIndexer | 61.59 us | 1.209 us | 1.952 us | 3.1128 | 3.0518 |  25.47 KB |

This gets *some* of the performance back, but not all.

The LOLs per second app, testing these changes on a Pixel 5:

    Before:
    376.98 LOLs/s
    After:
    391.44 LOLs/s
github-actions bot pushed a commit to jonathanpeppers/maui that referenced this issue Sep 22, 2023
Context: dotnet#13818 (review)
Context: jonathanpeppers/lols#4
Fixes: dotnet#17520

A customer noticed my LOLs per second sample was slower in .NET 8 than
.NET 7. I could reproduce their results.

Digging in, `dotnet-trace` showed one culprit was:

    .NET 7
     8.5% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
     1.2% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue
    .NET 8
    11.0% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
     2.8% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue

I knew that dotnet#13818 had some performance impact, as I noted when
reviewing the change.

Drilling in further, most of the time is spent calling
`SortedList.Last()`. Which makes sense, as `BindableObject.GetValue()`
is called *a lot* in a typical .NET MAUI application.

Adding some logging, I found my LOLs app most commonly had the following
specificity values when `BindableProperty`'s are set:

* 5,284 - a single specificity value
* 34,306 - two specificity values

No `BindableProperty`'s in this app had more than two specificity values.

So, an improvement here would be to:

* Avoid `SortedList` for the most common calls

* Make fields that store up to two specificity values

* If a *third* specificity value is required, fall back to using
`SortedList`.

I introduced a new, internal `SetterSpecificityList` class for this logic.

The results of running `BindingBenchmarker`:

    > .\bin\dotnet\dotnet.exe run --project .\src\Core\tests\Benchmarks\Core.Benchmarks.csproj -c Release -- --filter Microsoft.Maui.Benchmarks.BindingBenchmarker.*
    ...
    Before:
    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 31.67 us | 0.689 us | 2.009 us | 1.7395 | 1.7090 |  14.45 KB |
    |        BindChild | 42.18 us | 0.864 us | 2.548 us | 2.4414 | 2.3804 |  20.16 KB |
    | BindChildIndexer | 78.37 us | 1.564 us | 3.266 us | 3.5400 | 3.4180 |  29.69 KB |
    After:
    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 27.13 us | 0.521 us | 1.016 us | 1.3733 | 1.3428 |  11.33 KB |
    |        BindChild | 37.77 us | 0.845 us | 2.437 us | 2.0752 | 2.0142 |  17.03 KB |
    | BindChildIndexer | 69.45 us | 1.356 us | 2.859 us | 3.1738 | 3.0518 |  26.56 KB |

My original numbers (before specificity changes in dotnet#13818) were:

    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 24.46 us | 0.554 us | 1.624 us | 1.2512 | 1.2207 |  10.23 KB |
    |        BindChild | 33.21 us | 0.743 us | 2.192 us | 1.9226 | 1.8921 |  15.94 KB |
    | BindChildIndexer | 61.59 us | 1.209 us | 1.952 us | 3.1128 | 3.0518 |  25.47 KB |

This gets *some* of the performance back, but not all.

The LOLs per second app, testing these changes on a Pixel 5:

    Before:
    376.98 LOLs/s
    After:
    391.44 LOLs/s
PureWeen pushed a commit to dotnet/maui that referenced this issue Sep 23, 2023
Context: #13818 (review)
Context: jonathanpeppers/lols#4
Fixes: #17520

A customer noticed my LOLs per second sample was slower in .NET 8 than
.NET 7. I could reproduce their results.

Digging in, `dotnet-trace` showed one culprit was:

    .NET 7
     8.5% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
     1.2% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue
    .NET 8
    11.0% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.GetValue
     2.8% microsoft.maui.controls!Microsoft.Maui.Controls.BindableObject.SetValue

I knew that #13818 had some performance impact, as I noted when
reviewing the change.

Drilling in further, most of the time is spent calling
`SortedList.Last()`. Which makes sense, as `BindableObject.GetValue()`
is called *a lot* in a typical .NET MAUI application.

Adding some logging, I found my LOLs app most commonly had the following
specificity values when `BindableProperty`'s are set:

* 5,284 - a single specificity value
* 34,306 - two specificity values

No `BindableProperty`'s in this app had more than two specificity values.

So, an improvement here would be to:

* Avoid `SortedList` for the most common calls

* Make fields that store up to two specificity values

* If a *third* specificity value is required, fall back to using
`SortedList`.

I introduced a new, internal `SetterSpecificityList` class for this logic.

The results of running `BindingBenchmarker`:

    > .\bin\dotnet\dotnet.exe run --project .\src\Core\tests\Benchmarks\Core.Benchmarks.csproj -c Release -- --filter Microsoft.Maui.Benchmarks.BindingBenchmarker.*
    ...
    Before:
    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 31.67 us | 0.689 us | 2.009 us | 1.7395 | 1.7090 |  14.45 KB |
    |        BindChild | 42.18 us | 0.864 us | 2.548 us | 2.4414 | 2.3804 |  20.16 KB |
    | BindChildIndexer | 78.37 us | 1.564 us | 3.266 us | 3.5400 | 3.4180 |  29.69 KB |
    After:
    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 27.13 us | 0.521 us | 1.016 us | 1.3733 | 1.3428 |  11.33 KB |
    |        BindChild | 37.77 us | 0.845 us | 2.437 us | 2.0752 | 2.0142 |  17.03 KB |
    | BindChildIndexer | 69.45 us | 1.356 us | 2.859 us | 3.1738 | 3.0518 |  26.56 KB |

My original numbers (before specificity changes in #13818) were:

    |           Method |     Mean |    Error |   StdDev |   Gen0 |   Gen1 | Allocated |
    |----------------- |---------:|---------:|---------:|-------:|-------:|----------:|
    |         BindName | 24.46 us | 0.554 us | 1.624 us | 1.2512 | 1.2207 |  10.23 KB |
    |        BindChild | 33.21 us | 0.743 us | 2.192 us | 1.9226 | 1.8921 |  15.94 KB |
    | BindChildIndexer | 61.59 us | 1.209 us | 1.952 us | 3.1128 | 3.0518 |  25.47 KB |

This gets *some* of the performance back, but not all.

The LOLs per second app, testing these changes on a Pixel 5:

    Before:
    376.98 LOLs/s
    After:
    391.44 LOLs/s
@kcrg
Copy link
Author

kcrg commented Oct 10, 2023

@jonathanpeppers .NET 8 RC2 is definitely better than RC1, almost on par with .NET 7

.NET 8 RC2 .NET 7
Screenshot_20231011_014605 Screenshot_20231011_014546

@kcrg kcrg changed the title .NET 8 RC1 slower than .NET 7? .NET 8 slower than .NET 7? Oct 10, 2023
@jonathanpeppers
Copy link
Owner

Thanks, I believe this is probably where we are going to land for .NET 8 GA.

There is a community contribution to help make BindableProperty even better that could help even more on this one:

dotnet/maui#17756

But I think it will probably be .NET 8 servicing before this ships (if completed).

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

No branches or pull requests

2 participants