⚡ Optimize ModelManager deletion performance

osu.Game.Tests/Database/ModelManagerPerformanceTest.cs

@@ -0,0 +1,97 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Diagnostics;
+using System.Linq;
+using NUnit.Framework;
+using osu.Framework.Platform;
+using osu.Game.Beatmaps;
+using osu.Game.Database;
+using osu.Game.Rulesets;
+
+namespace osu.Game.Tests.Database
+{
+    [TestFixture]
+    public partial class ModelManagerPerformanceTest : RealmTest
+    {
+        [Test]
+        public void TestDeletePerformance()
+        {
+            RunTestWithRealm((realm, storage) =>
+            {
+                var manager = new TestModelManager(storage, realm);
+                const int count = 1000;
+
+                // Create items
+                realm.Write(r =>
+                {
+                    var ruleset = new RulesetInfo("osu", "osu!", string.Empty, 0) { Available = true };
+
+                    for (int i = 0; i < count; i++)
+                    {
+                        var set = CreateBeatmapSet(ruleset);
+                        r.Add(set);
+                    }
+                });
+
+                var items = realm.Run(r => r.All<BeatmapSetInfo>().ToList());
+                Assert.AreEqual(count, items.Count);
+
+                var sw = Stopwatch.StartNew();
+                manager.Delete(items);
+                sw.Stop();
+
+                Console.WriteLine($"Deleting {count} items took {sw.ElapsedMilliseconds}ms");
+
+                // Verify deletion
+                int remaining = realm.Run(r => r.All<BeatmapSetInfo>().Count(s => !s.DeletePending));
+                Assert.AreEqual(0, remaining);
+            });
+        }
+
+        [Test]
+        public void TestUndeletePerformance()
+        {
+            RunTestWithRealm((realm, storage) =>
+            {
+                var manager = new TestModelManager(storage, realm);
+                const int count = 1000;
+
+                // Create items and delete them
+                realm.Write(r =>
+                {
+                    var ruleset = new RulesetInfo("osu", "osu!", string.Empty, 0) { Available = true };
+
+                    for (int i = 0; i < count; i++)
+                    {
+                        var set = CreateBeatmapSet(ruleset);
+                        set.DeletePending = true;
+                        r.Add(set);
+                    }
+                });
+
+                var items = realm.Run(r => r.All<BeatmapSetInfo>().ToList());
+                Assert.AreEqual(count, items.Count);
+
+                var sw = Stopwatch.StartNew();
+                manager.Undelete(items);
+                sw.Stop();
+
+                Console.WriteLine($"Undeleting {count} items took {sw.ElapsedMilliseconds}ms");
+
+                // Verify undeletion
+                int remaining = realm.Run(r => r.All<BeatmapSetInfo>().Count(s => !s.DeletePending));
+                Assert.AreEqual(count, remaining);
+            });
+        }
+
+        private class TestModelManager : ModelManager<BeatmapSetInfo>
+        {
+            public TestModelManager(Storage storage, RealmAccess realm)
+                : base(storage, realm)
+            {
+            }
+        }
+    }
+}

osu.Game.Tests/Visual/Ranking/TestSceneSoloResultsScreen.cs

@@ -278,7 +278,7 @@ public void TestOnlineLeaderboardWithLessThan50Scores_ShowingAnotherUserScore()
 
             AddStep("show results", () => LoadScreen(new SoloResultsScreen(scores[0])));
             AddUntilStep("wait for loaded", () => ((Drawable)Stack.CurrentScreen).IsLoaded);
-            AddAssert("local user best shown", () => this.ChildrenOfType<ScorePanel>().Any(p => p.Score.UserID == API.LocalUser.Value.Id));
+            AddUntilStep("local user best shown", () => this.ChildrenOfType<ScorePanel>().Any(p => p.Score.UserID == API.LocalUser.Value.Id));
         }
 
         [Test]
@@ -535,7 +535,7 @@ public void TestOnlineLeaderboardDeduplication()
                 LoadScreen(new SoloResultsScreen(localScore));
             });
             AddUntilStep("wait for loaded", () => ((Drawable)Stack.CurrentScreen).IsLoaded);
-            AddAssert("only one score with ID 12345", () => this.ChildrenOfType<ScorePanel>().Count(s => s.Score.OnlineID == 12345), () => Is.EqualTo(1));
+            AddUntilStep("only one score with ID 12345", () => this.ChildrenOfType<ScorePanel>().Count(s => s.Score.OnlineID == 12345), () => Is.EqualTo(1));
             AddUntilStep("user best position preserved", () => this.ChildrenOfType<ScorePanel>().Any(p => p.ScorePosition.Value == 133_337));
         }
 

osu.Game/Database/ModelManager.cs

@@ -127,18 +127,21 @@ public void Delete(List<TModel> items, bool silent = false)
 
             int i = 0;
 
-            foreach (var b in items)
+            Realm.Write(realm =>
             {
-                if (notification.State == ProgressNotificationState.Cancelled)
-                    // user requested abort
-                    return;
+                foreach (var b in items)
+                {
+                    if (notification.State == ProgressNotificationState.Cancelled)
+                        // user requested abort
+                        return;
 
-                notification.Text = $"Deleting {HumanisedModelName}s ({++i} of {items.Count})";
+                    notification.Text = $"Deleting {HumanisedModelName}s ({++i} of {items.Count})";
 
-                Delete(b);
+                    Delete(b, realm);
 
-                notification.Progress = (float)i / items.Count;
-            }
+                    notification.Progress = (float)i / items.Count;
+                }
+            });
 
             notification.State = ProgressNotificationState.Completed;
         }
@@ -168,18 +171,21 @@ public void Undelete(List<TModel> items, bool silent = false)
 
             int i = 0;
 
-            foreach (var item in items)
+            Realm.Write(realm =>
             {
-                if (notification.State == ProgressNotificationState.Cancelled)
-                    // user requested abort
-                    return;
+                foreach (var item in items)
+                {
+                    if (notification.State == ProgressNotificationState.Cancelled)
+                        // user requested abort
+                        return;
 
-                notification.Text = $"Restoring ({++i} of {items.Count})";
+                    notification.Text = $"Restoring ({++i} of {items.Count})";
 
-                Undelete(item);
+                    Undelete(item, realm);
 
-                notification.Progress = (float)i / items.Count;
-            }
+                    notification.Progress = (float)i / items.Count;
+                }
+            });
 
             notification.State = ProgressNotificationState.Completed;
         }
@@ -188,35 +194,45 @@ public bool Delete(TModel item)
         {
             // Importantly, begin the realm write *before* re-fetching, else the update realm may not be in a consistent state
             // (ie. if an async import finished very recently).
-            return Realm.Write(realm =>
-            {
-                TModel? processableItem = item;
-                if (!processableItem.IsManaged)
-                    processableItem = realm.Find<TModel>(item.ID);
+            return Realm.Write(realm => Delete(item, realm));
+        }
+
+        /// <summary>
+        /// Delete an item from within an ongoing realm transaction.
+        /// </summary>
+        public bool Delete(TModel item, Realm realm)
+        {
+            TModel? processableItem = item;
+            if (!processableItem.IsManaged)
+                processableItem = realm.Find<TModel>(item.ID);
 
-                if (processableItem?.DeletePending != false)
-                    return false;
+            if (processableItem?.DeletePending != false)
+                return false;
 
-                processableItem.DeletePending = true;
-                return true;
-            });
+            processableItem.DeletePending = true;
+            return true;
         }
 
         public void Undelete(TModel item)
         {
             // Importantly, begin the realm write *before* re-fetching, else the update realm may not be in a consistent state
             // (ie. if an async import finished very recently).
-            Realm.Write(realm =>
-            {
-                TModel? processableItem = item;
-                if (!processableItem.IsManaged)
-                    processableItem = realm.Find<TModel>(item.ID);
+            Realm.Write(realm => Undelete(item, realm));
+        }
 
-                if (processableItem?.DeletePending != true)
-                    return;
+        /// <summary>
+        /// Undelete an item from within an ongoing realm transaction.
+        /// </summary>
+        public void Undelete(TModel item, Realm realm)
+        {
+            TModel? processableItem = item;
+            if (!processableItem.IsManaged)
+                processableItem = realm.Find<TModel>(item.ID);
 
-                processableItem.DeletePending = false;
-            });
+            if (processableItem?.DeletePending != true)
+                return;
+
+            processableItem.DeletePending = false;
         }
 
         public virtual bool IsAvailableLocally(TModel model) => true;