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

Improve realm update performance #30893

Merged
merged 1 commit into from
Nov 28, 2024
Merged

Conversation

peppy
Copy link
Member

@peppy peppy commented Nov 27, 2024

Coming from #28497.

Not sure why I left this around during the refactor. This is 100% handled by the DetachedBeatmapStore.

Removing this subscription reduces overheads by a huge amount for users with large beatmap databases. My hypothesis is that subscriptions are more expensive based on the number of results matching. This one matches almost every beatmap so removing it is a large win.

Using this patch to test:

diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
index dc13924b4f..11151104fc 100644
--- a/osu.Game/OsuGameBase.cs
+++ b/osu.Game/OsuGameBase.cs
@@ -419,6 +419,12 @@ private void load(ReadableKeyCombinationProvider keyCombinationProvider, Framewo
 
         private void updateLanguage() => CurrentLanguage.Value = LanguageExtensions.GetLanguageFor(frameworkLocale.Value, localisationParameters.Value);
 
+        protected override void Update()
+        {
+            base.Update();
+            realm.Run(r => r.Refresh());
+        }
+
         private void addFilesWarning()
         {
             var realmStore = new RealmFileStore(realm, Storage);

with this database provided here and profiling the overhead of importing a failed score:

Before After
Rider 2024-11-27 at 06 15 23 Rider 2024-11-27 at 06 13 50

Not sure why I left this around during the refactor. This is 100%
handled by the `DetachedBeatmapStore`.

Removing this subscription reduces overheads by a huge amount for users
with large beatmap databases. My hypothesis is that subscriptions are
more expensive based on **the number of results matching**. This one
matches almost every beatmap so removing it is a large win.
@peppy
Copy link
Member Author

peppy commented Nov 27, 2024

If we want to improve performance further (without completely pulling realm out), the next step would be to have a manual event flow which can trigger global invalidation on the DetachedBeatmapStore without requiring the realm pathway to be hit. But it may be agreed upon at that point that pulling out realm is in our best interest 🤷. Definite for another day.

@smoogipoo smoogipoo merged commit 1575eed into ppy:master Nov 28, 2024
7 of 10 checks passed
@peppy peppy deleted the realm-perf-improvements branch December 2, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
realm deals with local realm database size/M type:performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants