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

Reduce allocations during aggregate beatmap sorts in song-select screen #27173

Merged
merged 3 commits into from Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 35 additions & 7 deletions osu.Game/Screens/Select/Carousel/CarouselBeatmapSet.cs
Expand Up @@ -91,19 +91,19 @@ public override int CompareTo(FilterCriteria criteria, CarouselItem other)
break;

case SortMode.LastPlayed:
comparison = -compareUsingAggregateMax(otherSet, b => (b.LastPlayed ?? DateTimeOffset.MinValue).ToUnixTimeSeconds());
comparison = -compareUsingAggregateMax(otherSet, static b => (b.LastPlayed ?? DateTimeOffset.MinValue).ToUnixTimeSeconds());
break;

case SortMode.BPM:
comparison = compareUsingAggregateMax(otherSet, b => b.BPM);
comparison = compareUsingAggregateMax(otherSet, static b => b.BPM);
break;

case SortMode.Length:
comparison = compareUsingAggregateMax(otherSet, b => b.Length);
comparison = compareUsingAggregateMax(otherSet, static b => b.Length);
break;

case SortMode.Difficulty:
comparison = compareUsingAggregateMax(otherSet, b => b.StarRating);
comparison = compareUsingAggregateMax(otherSet, static b => b.StarRating);
break;

case SortMode.DateSubmitted:
Expand All @@ -127,12 +127,40 @@ public override int CompareTo(FilterCriteria criteria, CarouselItem other)
/// <summary>
/// All beatmaps which are not filtered and valid for display.
/// </summary>
protected IEnumerable<BeatmapInfo> ValidBeatmaps => Beatmaps.Where(b => !b.Filtered.Value || b.State.Value == CarouselItemState.Selected).Select(b => b.BeatmapInfo);
protected IEnumerable<BeatmapInfo> ValidBeatmaps
{
get
{
foreach (var item in Items) // iterating over Items directly to not allocate 2 enumerators
{
if (item is CarouselBeatmap b && (!b.Filtered.Value || b.State.Value == CarouselItemState.Selected))
yield return b.BeatmapInfo;
}
}
}

/// <summary>
/// Whether there are available beatmaps which are not filtered and valid for display.
/// Cheaper alternative to <see cref="ValidBeatmaps"/>.Any()
/// </summary>
public bool HasValidBeatmaps
{
get
{
foreach (var item in Items) // iterating over Items directly to not allocate 2 enumerators
{
if (item is CarouselBeatmap b && (!b.Filtered.Value || b.State.Value == CarouselItemState.Selected))
return true;
}

return false;
}
}

private int compareUsingAggregateMax(CarouselBeatmapSet other, Func<BeatmapInfo, double> func)
{
bool ourBeatmaps = ValidBeatmaps.Any();
bool otherBeatmaps = other.ValidBeatmaps.Any();
bool ourBeatmaps = HasValidBeatmaps;
bool otherBeatmaps = other.HasValidBeatmaps;

if (!ourBeatmaps && !otherBeatmaps) return 0;
if (!ourBeatmaps) return -1;
Expand Down
4 changes: 3 additions & 1 deletion osu.Game/Screens/Select/Carousel/CarouselGroup.cs
Expand Up @@ -2,6 +2,8 @@
// See the LICENCE file in the repository root for full licence text.

using System.Collections.Generic;
using osu.Framework.Extensions.ListExtensions;
using osu.Framework.Lists;

namespace osu.Game.Screens.Select.Carousel
{
Expand All @@ -12,7 +14,7 @@ public class CarouselGroup : CarouselItem
{
public override DrawableCarouselItem? CreateDrawableRepresentation() => null;

public IReadOnlyList<CarouselItem> Items => items;
public SlimReadOnlyListWrapper<CarouselItem> Items => items.AsSlimReadOnly();

public int TotalItemsNotFiltered { get; private set; }

Expand Down