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

Preliminary fixes for off-the-charts allocations #26422

Merged
merged 17 commits into from
Jan 9, 2024

Conversation

peppy
Copy link
Member

@peppy peppy commented Jan 7, 2024

These are just initial clean-ups to get them out of the way. I have more follow-up PRs incoming. The hope is that the changes in this PR should be very low-effort review/acceptance. I've saved anything which could cause issues for separate PRs.

And in case anyone wonders why I'm spending time on this, the reason is that there have been enough reports of stuttering in recent times to become concerning to me. During investigation I have been able to reproduce and it is related to GC. We have regressed heavily on allocations both due to new components and more animations than before. Currently we are doing 200k obj allocations per second during gameplay with argon. My immediate goal is to reduce that down by at least 90%.

Note that I am not paying any attention to allocation size, only object counts for now.


Testing done using dotmemory with a temporary project. I tried to hook dotmemory up to unit testing but I don't think it works on macOS / rider yet.

diff --git a/osu.Desktop.slnf b/osu.Desktop.slnf
index 503e5935f5..22005a17c5 100644
--- a/osu.Desktop.slnf
+++ b/osu.Desktop.slnf
@@ -2,6 +2,9 @@
   "solution": {
     "path": "osu.sln",
     "projects": [
+      "..\\osu-framework\\osu.Framework.NativeLibs\\osu.Framework.NativeLibs.csproj",
+      "..\\osu-framework\\osu.Framework\\osu.Framework.csproj",
+      "ConsoleApp2\\ConsoleApp2.csproj",
       "osu.Desktop\\osu.Desktop.csproj",
       "osu.Game.Benchmarks\\osu.Game.Benchmarks.csproj",
       "osu.Game.Rulesets.Catch.Tests\\osu.Game.Rulesets.Catch.Tests.csproj",
@@ -16,15 +19,14 @@
       "osu.Game.Tournament.Tests\\osu.Game.Tournament.Tests.csproj",
       "osu.Game.Tournament\\osu.Game.Tournament.csproj",
       "osu.Game\\osu.Game.csproj",
-
-      "Templates\\Rulesets\\ruleset-empty\\osu.Game.Rulesets.EmptyFreeform\\osu.Game.Rulesets.EmptyFreeform.csproj",
       "Templates\\Rulesets\\ruleset-empty\\osu.Game.Rulesets.EmptyFreeform.Tests\\osu.Game.Rulesets.EmptyFreeform.Tests.csproj",
-      "Templates\\Rulesets\\ruleset-example\\osu.Game.Rulesets.Pippidon\\osu.Game.Rulesets.Pippidon.csproj",
+      "Templates\\Rulesets\\ruleset-empty\\osu.Game.Rulesets.EmptyFreeform\\osu.Game.Rulesets.EmptyFreeform.csproj",
       "Templates\\Rulesets\\ruleset-example\\osu.Game.Rulesets.Pippidon.Tests\\osu.Game.Rulesets.Pippidon.Tests.csproj",
-      "Templates\\Rulesets\\ruleset-scrolling-empty\\osu.Game.Rulesets.EmptyScrolling\\osu.Game.Rulesets.EmptyScrolling.csproj",
+      "Templates\\Rulesets\\ruleset-example\\osu.Game.Rulesets.Pippidon\\osu.Game.Rulesets.Pippidon.csproj",
       "Templates\\Rulesets\\ruleset-scrolling-empty\\osu.Game.Rulesets.EmptyScrolling.Tests\\osu.Game.Rulesets.EmptyScrolling.Tests.csproj",
-      "Templates\\Rulesets\\ruleset-scrolling-example\\osu.Game.Rulesets.Pippidon\\osu.Game.Rulesets.Pippidon.csproj",
-      "Templates\\Rulesets\\ruleset-scrolling-example\\osu.Game.Rulesets.Pippidon.Tests\\osu.Game.Rulesets.Pippidon.Tests.csproj"
+      "Templates\\Rulesets\\ruleset-scrolling-empty\\osu.Game.Rulesets.EmptyScrolling\\osu.Game.Rulesets.EmptyScrolling.csproj",
+      "Templates\\Rulesets\\ruleset-scrolling-example\\osu.Game.Rulesets.Pippidon.Tests\\osu.Game.Rulesets.Pippidon.Tests.csproj",
+      "Templates\\Rulesets\\ruleset-scrolling-example\\osu.Game.Rulesets.Pippidon\\osu.Game.Rulesets.Pippidon.csproj"
     ]
   }
-}
+}
\ No newline at end of file
diff --git a/osu.Game/osu.Game.csproj b/osu.Game/osu.Game.csproj
index c7e0cc3808..83b125a9ef 100644
--- a/osu.Game/osu.Game.csproj
+++ b/osu.Game/osu.Game.csproj
@@ -36,7 +36,6 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
     <PackageReference Include="Realm" Version="11.5.0" />
-    <PackageReference Include="ppy.osu.Framework" Version="2023.1227.1" />
     <PackageReference Include="ppy.osu.Game.Resources" Version="2023.1228.0" />
     <PackageReference Include="Sentry" Version="3.40.0" />
     <!-- Held back due to 0.34.0 failing AOT compilation on ZstdSharp.dll dependency. -->
@@ -51,4 +50,7 @@
     <PackageReference Include="System.Runtime.InteropServices" Version="4.3.0" />
     <PackageReference Include="System.Runtime.Handles" Version="4.3.0" />
   </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\..\osu-framework\osu.Framework\osu.Framework.csproj" />
+  </ItemGroup>
 </Project>
diff --git a/osu.sln b/osu.sln
index aeec0843be..77a91eda44 100644
--- a/osu.sln
+++ b/osu.sln
@@ -95,6 +95,16 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Game.Rulesets.Pippidon"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Game.Rulesets.Pippidon.Tests", "Templates\Rulesets\ruleset-scrolling-example\osu.Game.Rulesets.Pippidon.Tests\osu.Game.Rulesets.Pippidon.Tests.csproj", "{1743BF7C-E6AE-4A06-BAD9-166D62894303}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Framework", "..\osu-framework\osu.Framework\osu.Framework.csproj", "{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Framework.NativeLibs", "..\osu-framework\osu.Framework.NativeLibs\osu.Framework.NativeLibs.csproj", "{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Framework.Android", "..\osu-framework\osu.Framework.Android\osu.Framework.Android.csproj", "{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "osu.Framework.iOS", "..\osu-framework\osu.Framework.iOS\osu.Framework.iOS.csproj", "{440EFCA2-AEF9-4016-8D68-3772D97CBC26}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConsoleApp2", "ConsoleApp2\ConsoleApp2.csproj", "{3642134C-8E35-4467-812B-8BF83C866AEB}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -537,6 +547,66 @@ Global
 		{1743BF7C-E6AE-4A06-BAD9-166D62894303}.Release|iPhone.Build.0 = Release|Any CPU
 		{1743BF7C-E6AE-4A06-BAD9-166D62894303}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
 		{1743BF7C-E6AE-4A06-BAD9-166D62894303}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Debug|iPhone.ActiveCfg = Debug|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Debug|iPhone.Build.0 = Debug|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Release|Any CPU.Build.0 = Release|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Release|iPhone.ActiveCfg = Release|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Release|iPhone.Build.0 = Release|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
+		{7B6532F9-29B5-4E9C-AE69-3D7BA47EDCD8}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Debug|iPhone.ActiveCfg = Debug|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Debug|iPhone.Build.0 = Debug|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Release|Any CPU.Build.0 = Release|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Release|iPhone.ActiveCfg = Release|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Release|iPhone.Build.0 = Release|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
+		{8DC0A213-8D1A-44A7-8EBB-F74A0EB30B88}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Debug|iPhone.ActiveCfg = Debug|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Debug|iPhone.Build.0 = Debug|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Release|Any CPU.Build.0 = Release|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Release|iPhone.ActiveCfg = Release|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Release|iPhone.Build.0 = Release|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
+		{34F372CE-2344-41DE-BD8A-A8A31EDE7E1B}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Debug|iPhone.ActiveCfg = Debug|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Debug|iPhone.Build.0 = Debug|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Release|Any CPU.Build.0 = Release|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Release|iPhone.ActiveCfg = Release|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Release|iPhone.Build.0 = Release|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
+		{440EFCA2-AEF9-4016-8D68-3772D97CBC26}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Debug|iPhone.ActiveCfg = Debug|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Debug|iPhone.Build.0 = Debug|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Debug|iPhoneSimulator.ActiveCfg = Debug|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Debug|iPhoneSimulator.Build.0 = Debug|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Release|iPhone.ActiveCfg = Release|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Release|iPhone.Build.0 = Release|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Release|iPhoneSimulator.ActiveCfg = Release|Any CPU
+		{3642134C-8E35-4467-812B-8BF83C866AEB}.Release|iPhoneSimulator.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

framework:

diff --git a/osu.Framework.Tests/Visual/Testing/TestSceneNestedGame.cs b/osu.Framework.Tests/Visual/Testing/TestSceneNestedGame.cs
index 59f5072dd..27f2c0290 100644
--- a/osu.Framework.Tests/Visual/Testing/TestSceneNestedGame.cs
+++ b/osu.Framework.Tests/Visual/Testing/TestSceneNestedGame.cs
@@ -81,7 +81,7 @@ public void TearDownSteps()
             AddStep("mark host running", () => hostWasRunningAfterNestedExit = true);
         }
 
-        protected override void RunTestsFromNUnit()
+        public override void RunTestsFromNUnit()
         {
             base.RunTestsFromNUnit();
 
diff --git a/osu.Framework/Testing/TestScene.cs b/osu.Framework/Testing/TestScene.cs
index a4c6e5576..62038ba83 100644
--- a/osu.Framework/Testing/TestScene.cs
+++ b/osu.Framework/Testing/TestScene.cs
@@ -491,7 +491,7 @@ public void SetUpTestForNUnit()
         }
 
         [TearDown]
-        protected virtual void RunTestsFromNUnit()
+        public virtual void RunTestsFromNUnit()
         {
             RunTearDownSteps();
 

Red underlines show the overhead removed by the commit written below. Note that the total objects allocated is misleading as I was doing 5 second time samples, and as I optimised the whole tests would run faster.

2024-01-07 05 29 26@2x

91bb3f6 Cache argon character glyph lookups to reduce string allocations
5b55ca6 Cache legacy skin character glyph lookups to reduce string allocations

2024-01-07 05 25 57@2x

e9289cf Reduce precision of audio balance adjustments during slider sliding

2024-01-07 05 21 39@2x

b809d4c Remove delegate overhead from argon health display's animation updates

Note that I pulled part of the change I had here because I'll be addressing in a separate pull.

2024-01-07 05 19 11@2x

9d9e6fc Remove LINQ calls in hot paths

2024-01-07 05 15 18@2x

35eff63 Remove unnecessary second iteration over NestedHitObjects

2024-01-07 05 10 57@2x

5cc4a58 Avoid iteration over NestedHitObjects in silder's Update unless necessary

2024-01-07 05 09 48@2x

16ea7f9 Avoid completely unnecessary string allocations in ArgonCounterTextComponent

2024-01-07 13 10 29@2x

(framework input sampler api change)

2024-01-07 13 26 33@2x

@peppy peppy force-pushed the allocs-off-the-charts branch from 02a4a3e to 962c8ba Compare January 7, 2024 11:55
@bdach bdach self-requested a review January 8, 2024 08:48
@bdach bdach added the next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! label Jan 8, 2024
@peppy peppy force-pushed the allocs-off-the-charts branch from b99c9ab to 765d41f Compare January 9, 2024 05:07
@peppy
Copy link
Member Author

peppy commented Jan 9, 2024

Just as a heads up my OP diff is incomplete. I'll post an updated one when I'm at my other PC, if required.

@bdach bdach self-requested a review January 9, 2024 10:07
@bdach
Copy link
Collaborator

bdach commented Jan 9, 2024

Fixed a few more missing wireframes, rest seems fine

@bdach bdach enabled auto-merge January 9, 2024 13:12
@bdach
Copy link
Collaborator

bdach commented Jan 9, 2024

build canceled because github actions is dying of death, i intend to wait for recovery

@bdach bdach disabled auto-merge January 9, 2024 15:06
@bdach bdach merged commit 8e133ed into ppy:master Jan 9, 2024
@peppy peppy deleted the allocs-off-the-charts branch January 12, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! size/L type:performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants