perf: update framework submodule + optimize gameplay hot paths - #216
Conversation
…th optimizations and bug fixes Updates include: - Fix NullReferenceException in GraphicsPipeline.cs (ResourceLayouts?.Length) - Fix null safety in VeldridExtensions.Clone() (ResourceLayouts null check) - Fix null-forgiving operators in shader compilation (ShaderCompilationStore/VeldridShader) - Performance optimizations: HashSet for hover tracking, LazyList struct enumerator - Performance optimizations round 2: hot-path LINQ removal, array stats, single-pass trim - Fix 30 CS1591 warnings from veldrid submodule + IL trimmer fixes - Fix NuGet pack warnings for iOS project (NU5100, NU5118) - Fix workflow failures: update action versions, fix shallow clone - Remove redundant nullable operators per InspectCode warnings Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…aySampleTriggerSource, HitEventExtensions Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/adbddea2-3da1-47fd-b556-24d1a5d61ad5 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/adbddea2-3da1-47fd-b556-24d1a5d61ad5 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…ObjectContainer; replace Single/SingleOrDefault with First/FirstOrDefault; fix OnlineStatusNotifier bindable leak Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/b0878070-74cc-41e1-af2d-af6fb8954594 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…olourHitErrorMeter hot paths Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/b0878070-74cc-41e1-af2d-af6fb8954594 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/f395389a-2ef2-4bd0-a136-d6f868caceb2 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…kage caching for all jobs Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/ef1cac07-0e5c-4e78-80ca-6affd0c0fd6b Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
… version computation Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/1eaec755-4862-4a45-a4e9-3ab598e3840d Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
| { | ||
| base.Dispose(isDisposing); | ||
|
|
||
| apiState?.UnbindAll(); |
| base.Dispose(isDisposing); | ||
|
|
||
| apiState?.UnbindAll(); | ||
| multiplayerState?.UnbindAll(); |
|
|
||
| apiState?.UnbindAll(); | ||
| multiplayerState?.UnbindAll(); | ||
| spectatorState?.UnbindAll(); |
…ifier.Dispose (InspectCode warning) Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/78598192-f2c2-49e8-9b77-ba2c9587efa7 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR focuses on reducing allocations and avoiding repeated LINQ enumeration in gameplay/editor hot paths, alongside a small CI improvement and a cleanup to address an InspectCode warning.
Changes:
- Optimize several per-frame/per-judgement paths by replacing LINQ chains with single-pass loops or cheaper enumerations.
- Improve
HitObjectContainerordering accessors to avoid sorting on everyObjects/AliveObjectsaccess. - Add NuGet package caching to CI jobs and adjust
OnlineStatusNotifier.Dispose()unbinding.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| osu.Game/Screens/Play/HUD/HitErrorMeters/ColourHitErrorMeter.cs | Avoids repeated LINQ re-enumeration when trimming excess judgements. |
| osu.Game/Screens/Play/HUD/ClicksPerSecond/ClicksPerSecondController.cs | Reworks rewind trimming/counting to reduce per-element removals. |
| osu.Game/Screens/Edit/GameplayTest/EditorPlayer.cs | Uses FirstOrDefault() instead of SingleOrDefault() to reduce enumeration cost. |
| osu.Game/Screens/Edit/Compose/HitObjectUsageEventBuffer.cs | Uses First() instead of Single() to reduce enumeration cost. |
| osu.Game/Rulesets/UI/HitObjectContainer.cs | Avoids OrderBy() on every access; uses reverse enumeration + in-place sort for alive objects. |
| osu.Game/Rulesets/UI/GameplaySampleTriggerSource.cs | Removes LINQ-heavy selection logic in favour of single-pass scans. |
| osu.Game/Rulesets/UI/DrawableRuleset.cs | Uses FirstOrDefault() instead of SingleOrDefault() to reduce enumeration cost. |
| osu.Game/Rulesets/Scoring/HitEventExtensions.cs | Computes average hit error via running sum to avoid intermediate array allocation. |
| osu.Game/Online/OnlineStatusNotifier.cs | Adjusts bindable unbinding in Dispose() (per InspectCode warning). |
| .github/workflows/ci.yml | Adds NuGet package caching to speed up restores across CI jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Timestamps are added in chronological order (from clock.CurrentTime), | ||
| // so we can use binary-search-style trimming instead of per-element RemoveAt. | ||
|
|
||
| // Trim future timestamps caused by rewinding (remove from the end in one batch). | ||
| // RemoveRange from the end is a single operation vs repeated RemoveAt calls. | ||
| int trimStart = timestamps.Count; | ||
|
|
||
| while (trimStart > 0 && timestamps[trimStart - 1] > latestValidTime) | ||
| trimStart--; | ||
|
|
||
| if (trimStart < timestamps.Count) | ||
| timestamps.RemoveRange(trimStart, timestamps.Count - trimStart); | ||
|
|
There was a problem hiding this comment.
The new trimming/counting logic assumes timestamps is always in non-decreasing order and that any "future" timestamps (from rewinds/seeks) are contiguous at the end. If an input timestamp is added after a seek/rewind but before the next Update() (or if replay input is re-applied after rewinding), the list can become out-of-order and this RemoveRange-from-end pass will leave future timestamps earlier in the list, inflating the CPS count. Consider either maintaining sorted order on insertion (e.g. insert at the correct position) or falling back to removing future timestamps during the backwards scan when an out-of-order condition is detected (or clearing the list on rewind/seek).
| apiState.UnbindAll(); | ||
| multiplayerState.UnbindAll(); | ||
| spectatorState.UnbindAll(); |
There was a problem hiding this comment.
These bindables are initialised in load() via GetBoundCopy(), but Dispose() can run even if load() never executed (note the existing IsNotNull() guards for clients). Calling UnbindAll() unconditionally here can therefore throw if any of these fields are still null at disposal time. Consider initialising them at declaration (e.g. new Bindable<...>() + BindTo(...) in load()), or making them nullable and using null-conditional unbinds.
| apiState.UnbindAll(); | |
| multiplayerState.UnbindAll(); | |
| spectatorState.UnbindAll(); | |
| apiState?.UnbindAll(); | |
| multiplayerState?.UnbindAll(); | |
| spectatorState?.UnbindAll(); |
apiState?.,multiplayerState?.,spectatorState?.use null-conditional on non-nullable fields?.→.for all three UnbindAll() calls in DisposeSummary by Gitar
OnlineStatusNotifier.Disposeto resolveInspectCodewarnings.This will update automatically on new commits.