Merge pull request #16510 from peppy/optimise-set-detach-part-2

Avoid file retrieval overhead when detaching `BeatmapSetInfo`

osu.Game.Tests/Database/BeatmapImporterTests.cs

@@ -57,8 +57,9 @@ namespace osu.Game.Tests.Database
                     {
                         detachedBeatmapSet = live.Detach();
 
-                        Assert.AreEqual(live.Files.Count, detachedBeatmapSet.Files.Count);
+                        // files are omitted
-                        Assert.AreEqual(live.Files.Select(f => f.File).Count(), detachedBeatmapSet.Files.Select(f => f.File).Count());
+                        Assert.AreEqual(0, detachedBeatmapSet.Files.Count);
+
                         Assert.AreEqual(live.Beatmaps.Count, detachedBeatmapSet.Beatmaps.Count);
                         Assert.AreEqual(live.Beatmaps.Select(f => f.Difficulty).Count(), detachedBeatmapSet.Beatmaps.Select(f => f.Difficulty).Count());
                         Assert.AreEqual(live.Metadata, detachedBeatmapSet.Metadata);
@@ -67,11 +68,9 @@ namespace osu.Game.Tests.Database
                     Debug.Assert(detachedBeatmapSet != null);
 
                     // Check detached instances can all be accessed without throwing.
-                    Assert.NotNull(detachedBeatmapSet.Files.Count);
+                    Assert.AreEqual(0, detachedBeatmapSet.Files.Count);
-                    Assert.NotZero(detachedBeatmapSet.Files.Select(f => f.File).Count());
                     Assert.NotNull(detachedBeatmapSet.Beatmaps.Count);
                     Assert.NotZero(detachedBeatmapSet.Beatmaps.Select(f => f.Difficulty).Count());
-                    Assert.NotNull(detachedBeatmapSet.Beatmaps.First().Path);
                     Assert.NotNull(detachedBeatmapSet.Metadata);
 
                     // Check cyclic reference to beatmap set
@@ -96,9 +95,12 @@ namespace osu.Game.Tests.Database
                     Assert.NotNull(beatmapSet);
                     Debug.Assert(beatmapSet != null);
 
-                    BeatmapSetInfo? detachedBeatmapSet = null;
+                    // Detach at the BeatmapInfo point, similar to what GetWorkingBeatmap does.
+                    BeatmapInfo? detachedBeatmap = null;
 
-                    beatmapSet.PerformRead(s => detachedBeatmapSet = s.Detach());
+                    beatmapSet.PerformRead(s => detachedBeatmap = s.Beatmaps.First().Detach());
+
+                    BeatmapSetInfo? detachedBeatmapSet = detachedBeatmap?.BeatmapSet;
 
                     Debug.Assert(detachedBeatmapSet != null);
 

osu.Game/Beatmaps/BeatmapManager.cs

@@ -299,7 +299,23 @@ namespace osu.Game.Beatmaps
 
         #region Implementation of IWorkingBeatmapCache
 
-        public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo? importedBeatmap) => workingBeatmapCache.GetWorkingBeatmap(importedBeatmap);
+        public WorkingBeatmap GetWorkingBeatmap(BeatmapInfo? importedBeatmap)
+        {
+            // Detached sets don't come with files.
+            // If we seem to be missing files, now is a good time to re-fetch.
+            if (importedBeatmap?.BeatmapSet?.Files.Count == 0)
+            {
+                using (var realm = contextFactory.CreateContext())
+                {
+                    var refetch = realm.Find<BeatmapInfo>(importedBeatmap.ID)?.Detach();
+
+                    if (refetch != null)
+                        importedBeatmap = refetch;
+                }
+            }
+
+            return workingBeatmapCache.GetWorkingBeatmap(importedBeatmap);
+        }
 
         public WorkingBeatmap GetWorkingBeatmap(ILive<BeatmapInfo>? importedBeatmap)
         {

osu.Game/Database/RealmObjectExtensions.cs

@@ -76,6 +76,14 @@ namespace osu.Game.Database
         {
             applyCommonConfiguration(c);
 
+            c.CreateMap<BeatmapSetInfo, BeatmapSetInfo>()
+             .MaxDepth(2)
+             .AfterMap((s, d) =>
+             {
+                 foreach (var beatmap in d.Beatmaps)
+                     beatmap.BeatmapSet = d;
+             });
+
             // This can be further optimised to reduce cyclic retrievals, similar to the optimised set mapper below.
             // Only hasn't been done yet as we detach at the point of BeatmapInfo less often.
             c.CreateMap<BeatmapInfo, BeatmapInfo>()
@@ -100,6 +108,15 @@ namespace osu.Game.Database
         {
             applyCommonConfiguration(c);
 
+            c.CreateMap<BeatmapSetInfo, BeatmapSetInfo>()
+             .MaxDepth(2)
+             .ForMember(b => b.Files, cc => cc.Ignore())
+             .AfterMap((s, d) =>
+             {
+                 foreach (var beatmap in d.Beatmaps)
+                     beatmap.BeatmapSet = d;
+             });
+
             c.CreateMap<BeatmapInfo, BeatmapInfo>()
              .MaxDepth(1)
              // This is not required as it will be populated in the `AfterMap` call from the `BeatmapInfo`'s parent.
@@ -132,13 +149,6 @@ namespace osu.Game.Database
             c.CreateMap<RealmUser, RealmUser>();
             c.CreateMap<RealmFile, RealmFile>();
             c.CreateMap<RealmNamedFileUsage, RealmNamedFileUsage>();
-            c.CreateMap<BeatmapSetInfo, BeatmapSetInfo>()
-             .MaxDepth(2)
-             .AfterMap((s, d) =>
-             {
-                 foreach (var beatmap in d.Beatmaps)
-                     beatmap.BeatmapSet = d;
-             });
         }
 
         /// <summary>