Fix song select scrolling performance when user has many beatmaps loaded#37666
Merged
bdach merged 5 commits intoppy:masterfrom May 8, 2026
Merged
Fix song select scrolling performance when user has many beatmaps loaded#37666bdach merged 5 commits intoppy:masterfrom
bdach merged 5 commits intoppy:masterfrom
Conversation
The real killer here is filters on linked objects. Use the knowledge that filtering down to a beatmap's full scores is already quite refined, and move the slow parts out to post-filtering.
Already fast enough without this that I cannot see a difference, but best to have it in place.
peppy
changed the title
bdach
reviewed
May 8, 2026
| /// 51 2025-07-22 Add ScoreInfo.Pauses. | ||
| /// </summary> | ||
| private const int schema_version = 51; | ||
| private const int schema_version = 52; |
Collaborator
There was a problem hiding this comment.
Is there a reason to bump schema version?
Adding [Indexed] to a property shouldn't require a schema version bump as per realm/realm-dotnet#3370 (comment).
Member
Author
There was a problem hiding this comment.
Yeah okay, I seem to recall that being a thing but wasn't sure. In which case this isn't required.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bdach
reviewed
May 8, 2026
Collaborator
|
After some further staring I'm still not convinced that That said, my one-shot imprecise profiling results are not exactly super conclusive other than saying that pretty much any variant I jiggled towards is faster than |
bdach
approved these changes
May 8, 2026
Member
Author
FWIW my best profiling was just scrolling in song select with that database in "sort by difficulty" (splitting out individual difficulties) and watching for scheduler spikes, as per videos in OP. You can test with the original method and the modified one and see a diferenence of 2-10 milliseconds, hopefully. |
ShikkesoraSIM
added a commit
to ShikkesoraSIM/torii-osu
that referenced
this pull request
May 8, 2026
Backports the song-select scrolling perf fix landed upstream as ppy/osu#37666. Original symptom: scrolling song select with a large beatmap library spent 2-10ms per tick inside Realm running a filter on linked objects (`ScoreInfo.BeatmapInfo.ID` + `ScoreInfo.Ruleset.ShortName`), because Realm has to walk each candidate's linked rows to evaluate those predicates. The rewrite moves the filter shape from "linked-object filter inside Realm" to "flat field filter inside Realm + LINQ narrow in .NET": - Realm subscription now matches on the flat `BeatmapHash` column only (and `!s.DeletePending`). Realm can satisfy this without walking links. - The ruleset / user-id narrowing happens post-fetch on the much smaller candidate set, in plain LINQ. - Mirror upstream's `setRankFromScore(null)` reset on subscription re-arm — fixes the secondary bug ppy/osu#37661 where the previous beatmap's rank was briefly visible during transitions. Diverges from upstream in one deliberate way: we DO NOT add the `[Indexed]` attribute on `ScoreInfo.BeatmapHash` that the original PR includes for an extra ~10-30% bump on the same query. Adding it would require bumping the Realm schema 51 → 52, which Torii explicitly avoids to preserve compatibility with vanilla osu! lazer opening shared realm folders (see RealmAccess.cs schema_version comment + RealmDowngrader). The filter rewrite alone — which the PR description calls "the real killer" — applies cleanly here. Test step also ported: TestLocalRank now starts from a null-rank state so transitions out of "no rank yet" are part of the regression matrix.
ShikkesoraSIM
added a commit
to ShikkesoraSIM/torii-osu
that referenced
this pull request
May 8, 2026
ShikkesoraSIM
added a commit
to ShikkesoraSIM/torii-osu
that referenced
this pull request
May 8, 2026
…s + screen context
Turns the hiccup dashboard from "Unknown — see context fields" noise
into actionable observability. Per-record context now includes screen
+ visible overlays + the last 16 cross-codebase events leading up to
the stall, and the cause heuristic chooses the freshest correlated
breadcrumb instead of just falling back to "Unknown".
A typical post-A hiccup record reads as:
{
"frame_ms": 187.4,
"current_screen": "SongSelectV2",
"visible_overlays": ["ChatOverlay"],
"likely_cause": "Realm query on update thread 50 ms ago — update-thread (87 ms)",
"recent_events": [
{"kind": "input.click", "detail": "..."},
{"kind": "carousel.filter", "detail": "text='abc' reset=true"},
{"kind": "realm.run", "detail": "update-thread (87 ms)"},
{"kind": "api.request.start", "detail": "GET /scores/123"}
],
...
}
Pieces shipped
--------------
* `HiccupBreadcrumbs` static sink (new). Single null-check fast path
when the logger is OFF — sprinkling `HiccupBreadcrumbs.Add(...)`
across hot paths costs nothing in production builds. Volatile
reference cell + ring-buffer atomic increment on the hot path; safe
to call from any thread. The logger registers itself on load and
unregisters on dispose.
* `OsuGame.CurrentTopScreen` + `OsuGame.RegisteredFocusedOverlays`
internal accessors. Read-only views over the existing screen stack
+ focused-overlay list, so the logger can capture screen + visible-
overlays context without reflection or breaking encapsulation.
* Hookpoints emitting breadcrumbs:
* Screen push / exit (OsuGame.screenPushed / screenExited).
* Overlay show / hide (every OsuFocusedOverlayContainer registered
via either RegisterBlockingOverlay or loadComponentSingleFile —
hookOverlayBreadcrumbs binds to State changes).
* API requests, both start + end with duration (APIRequest.Perform).
Catches sync waits that would show as "request in flight" at
hiccup time.
* RealmAccess.Run / Run<T> on the update thread, with duration ≥1ms.
Catches the realm-query-stalls-the-UI-thread pattern that
ppy/osu#37666 fixed for the song-select rank panel.
* BeatmapCarousel.Filter — first instrumented operation in the
song-select hot path; more carousel hooks to come.
* BeatmapManager / ScoreManager / SkinManager PostNotification —
proxies user-visible notifications into breadcrumbs so an import
/ score completion / skin change is visible in the recent-events
window.
* Threshold default bumped 33 ms → 80 ms. The 33-ms floor was
capturing every borderline frame (vsync misses, harmless Gen0 GCs)
and flooding the dashboard with noise. 80 ms means "the user
definitely felt this stutter". Configurable as before via the
ThresholdMs property; the constant is the default-when-not-set.
* `guessCause` rewritten. Walks the recent-events ring buffer newest
first within a 500 ms correlation window and picks the most
specific breadcrumb-driven reason. Falls back to GC counts and
frame_ms only when no breadcrumb explains the stall. Reads like a
switch over kinds — easy to extend with more breadcrumb sources.
* `currentScreenName` + `collectVisibleOverlays` now actually return
values (the v1 stubs returned null because the tree-walk was
blocked by `protected` InternalChildren). They read from OsuGame's
new internal accessors instead.
What's NOT in this commit (deferred)
------------------------------------
Path B (stack-trace sampling on stalls) needs more investigation
than fits cleanly into one session. The right approach is probably
EventPipe via DiagnosticsClient + the TraceEvent parsing library,
which is a ~25MB dep we'd be adding to the desktop bundle. Path A
ships first; if data quality is still insufficient after we have
real breadcrumb-flavoured records flowing in, we revisit.
Doesn't touch the existing client uploader pipeline, the server
ingest endpoint, or the dashboard UI — Path A's record fields are
already on the schema (recent_events / current_screen /
visible_overlays were nullable strings / json on the table from the
v1 ingest commit). The dashboard's modal already renders them; with
Path A populated they'll just suddenly have content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Also fixes wrong rank showing briefly in some scenarios.
I'm quite confused why the overhead is in the post-async-filter collection access, but it is. It occurs when using
MaxBy(realm snapshot creation), but also when calling.Counton the collection, or even just accessingsender[0]. I tried everything, and ended up settling on simplifying the realm part enough that we can do post-filtering without much sweat or mess.Before:
osu.2026-05-07.at.11.41.58.mp4
After:
osu.2026-05-07.at.11.41.17.mp4
Tested using this realm.