1
0
mirror of https://github.com/ppy/osu.git synced 2024-12-14 21:02:55 +08:00

Merge pull request #18619 from peppy/fix-collection-performance

Fix performance overhead of large collections
This commit is contained in:
Dan Balasescu 2022-06-10 15:01:31 +09:00 committed by GitHub
commit c9dfffbc0e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 59 additions and 50 deletions

View File

@ -49,11 +49,16 @@ namespace osu.Game.Tests.Collections.IO
Assert.That(osu.CollectionManager.Collections.Count, Is.EqualTo(2)); Assert.That(osu.CollectionManager.Collections.Count, Is.EqualTo(2));
// Even with no beatmaps imported, collections are tracking the hashes and will continue to.
// In the future this whole mechanism will be replaced with having the collections in realm,
// but until that happens it makes rough sense that we want to track not-yet-imported beatmaps
// and have them associate with collections if/when they become available.
Assert.That(osu.CollectionManager.Collections[0].Name.Value, Is.EqualTo("First")); Assert.That(osu.CollectionManager.Collections[0].Name.Value, Is.EqualTo("First"));
Assert.That(osu.CollectionManager.Collections[0].Beatmaps.Count, Is.Zero); Assert.That(osu.CollectionManager.Collections[0].BeatmapHashes.Count, Is.EqualTo(1));
Assert.That(osu.CollectionManager.Collections[1].Name.Value, Is.EqualTo("Second")); Assert.That(osu.CollectionManager.Collections[1].Name.Value, Is.EqualTo("Second"));
Assert.That(osu.CollectionManager.Collections[1].Beatmaps.Count, Is.Zero); Assert.That(osu.CollectionManager.Collections[1].BeatmapHashes.Count, Is.EqualTo(12));
} }
finally finally
{ {
@ -76,10 +81,10 @@ namespace osu.Game.Tests.Collections.IO
Assert.That(osu.CollectionManager.Collections.Count, Is.EqualTo(2)); Assert.That(osu.CollectionManager.Collections.Count, Is.EqualTo(2));
Assert.That(osu.CollectionManager.Collections[0].Name.Value, Is.EqualTo("First")); Assert.That(osu.CollectionManager.Collections[0].Name.Value, Is.EqualTo("First"));
Assert.That(osu.CollectionManager.Collections[0].Beatmaps.Count, Is.EqualTo(1)); Assert.That(osu.CollectionManager.Collections[0].BeatmapHashes.Count, Is.EqualTo(1));
Assert.That(osu.CollectionManager.Collections[1].Name.Value, Is.EqualTo("Second")); Assert.That(osu.CollectionManager.Collections[1].Name.Value, Is.EqualTo("Second"));
Assert.That(osu.CollectionManager.Collections[1].Beatmaps.Count, Is.EqualTo(12)); Assert.That(osu.CollectionManager.Collections[1].BeatmapHashes.Count, Is.EqualTo(12));
} }
finally finally
{ {
@ -142,8 +147,8 @@ namespace osu.Game.Tests.Collections.IO
await importCollectionsFromStream(osu, TestResources.OpenResource("Collections/collections.db")); await importCollectionsFromStream(osu, TestResources.OpenResource("Collections/collections.db"));
// Move first beatmap from second collection into the first. // Move first beatmap from second collection into the first.
osu.CollectionManager.Collections[0].Beatmaps.Add(osu.CollectionManager.Collections[1].Beatmaps[0]); osu.CollectionManager.Collections[0].BeatmapHashes.Add(osu.CollectionManager.Collections[1].BeatmapHashes[0]);
osu.CollectionManager.Collections[1].Beatmaps.RemoveAt(0); osu.CollectionManager.Collections[1].BeatmapHashes.RemoveAt(0);
// Rename the second collecction. // Rename the second collecction.
osu.CollectionManager.Collections[1].Name.Value = "Another"; osu.CollectionManager.Collections[1].Name.Value = "Another";
@ -164,10 +169,10 @@ namespace osu.Game.Tests.Collections.IO
Assert.That(osu.CollectionManager.Collections.Count, Is.EqualTo(2)); Assert.That(osu.CollectionManager.Collections.Count, Is.EqualTo(2));
Assert.That(osu.CollectionManager.Collections[0].Name.Value, Is.EqualTo("First")); Assert.That(osu.CollectionManager.Collections[0].Name.Value, Is.EqualTo("First"));
Assert.That(osu.CollectionManager.Collections[0].Beatmaps.Count, Is.EqualTo(2)); Assert.That(osu.CollectionManager.Collections[0].BeatmapHashes.Count, Is.EqualTo(2));
Assert.That(osu.CollectionManager.Collections[1].Name.Value, Is.EqualTo("Another")); Assert.That(osu.CollectionManager.Collections[1].Name.Value, Is.EqualTo("Another"));
Assert.That(osu.CollectionManager.Collections[1].Beatmaps.Count, Is.EqualTo(11)); Assert.That(osu.CollectionManager.Collections[1].BeatmapHashes.Count, Is.EqualTo(11));
} }
finally finally
{ {

View File

@ -152,7 +152,7 @@ namespace osu.Game.Tests.Visual.Collections
AddStep("add two collections with same name", () => manager.Collections.AddRange(new[] AddStep("add two collections with same name", () => manager.Collections.AddRange(new[]
{ {
new BeatmapCollection { Name = { Value = "1" } }, new BeatmapCollection { Name = { Value = "1" } },
new BeatmapCollection { Name = { Value = "1" }, Beatmaps = { beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0] } }, new BeatmapCollection { Name = { Value = "1" }, BeatmapHashes = { beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0].MD5Hash } },
})); }));
} }
@ -162,7 +162,7 @@ namespace osu.Game.Tests.Visual.Collections
AddStep("add two collections", () => manager.Collections.AddRange(new[] AddStep("add two collections", () => manager.Collections.AddRange(new[]
{ {
new BeatmapCollection { Name = { Value = "1" } }, new BeatmapCollection { Name = { Value = "1" } },
new BeatmapCollection { Name = { Value = "2" }, Beatmaps = { beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0] } }, new BeatmapCollection { Name = { Value = "2" }, BeatmapHashes = { beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0].MD5Hash } },
})); }));
assertCollectionCount(2); assertCollectionCount(2);
@ -198,7 +198,7 @@ namespace osu.Game.Tests.Visual.Collections
{ {
AddStep("add two collections", () => manager.Collections.AddRange(new[] AddStep("add two collections", () => manager.Collections.AddRange(new[]
{ {
new BeatmapCollection { Name = { Value = "1" }, Beatmaps = { beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0] } }, new BeatmapCollection { Name = { Value = "1" }, BeatmapHashes = { beatmapManager.GetAllUsableBeatmapSets().First().Beatmaps[0].MD5Hash } },
})); }));
assertCollectionCount(1); assertCollectionCount(1);

View File

@ -151,10 +151,10 @@ namespace osu.Game.Tests.Visual.SongSelect
AddStep("add collection", () => collectionManager.Collections.Add(new BeatmapCollection { Name = { Value = "1" } })); AddStep("add collection", () => collectionManager.Collections.Add(new BeatmapCollection { Name = { Value = "1" } }));
AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare)); AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
AddStep("add beatmap to collection", () => collectionManager.Collections[0].Beatmaps.Add(Beatmap.Value.BeatmapInfo)); AddStep("add beatmap to collection", () => collectionManager.Collections[0].BeatmapHashes.Add(Beatmap.Value.BeatmapInfo.MD5Hash));
AddAssert("button is minus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.MinusSquare)); AddAssert("button is minus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.MinusSquare));
AddStep("remove beatmap from collection", () => collectionManager.Collections[0].Beatmaps.Clear()); AddStep("remove beatmap from collection", () => collectionManager.Collections[0].BeatmapHashes.Clear());
AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare)); AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
} }
@ -169,11 +169,11 @@ namespace osu.Game.Tests.Visual.SongSelect
AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare)); AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
addClickAddOrRemoveButtonStep(1); addClickAddOrRemoveButtonStep(1);
AddAssert("collection contains beatmap", () => collectionManager.Collections[0].Beatmaps.Contains(Beatmap.Value.BeatmapInfo)); AddAssert("collection contains beatmap", () => collectionManager.Collections[0].BeatmapHashes.Contains(Beatmap.Value.BeatmapInfo.MD5Hash));
AddAssert("button is minus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.MinusSquare)); AddAssert("button is minus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.MinusSquare));
addClickAddOrRemoveButtonStep(1); addClickAddOrRemoveButtonStep(1);
AddAssert("collection does not contain beatmap", () => !collectionManager.Collections[0].Beatmaps.Contains(Beatmap.Value.BeatmapInfo)); AddAssert("collection does not contain beatmap", () => !collectionManager.Collections[0].BeatmapHashes.Contains(Beatmap.Value.BeatmapInfo.MD5Hash));
AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare)); AddAssert("button is plus", () => getAddOrRemoveButton(1).Icon.Equals(FontAwesome.Solid.PlusSquare));
} }

View File

@ -90,6 +90,7 @@ namespace osu.Game.Beatmaps
public double StarRating { get; set; } public double StarRating { get; set; }
[Indexed]
public string MD5Hash { get; set; } = string.Empty; public string MD5Hash { get; set; } = string.Empty;
[JsonIgnore] [JsonIgnore]

View File

@ -23,9 +23,9 @@ namespace osu.Game.Collections
public readonly Bindable<string> Name = new Bindable<string>(); public readonly Bindable<string> Name = new Bindable<string>();
/// <summary> /// <summary>
/// The beatmaps contained by the collection. /// The <see cref="BeatmapInfo.MD5Hash"/>es of beatmaps contained by the collection.
/// </summary> /// </summary>
public readonly BindableList<BeatmapInfo> Beatmaps = new BindableList<BeatmapInfo>(); public readonly BindableList<string> BeatmapHashes = new BindableList<string>();
/// <summary> /// <summary>
/// The date when this collection was last modified. /// The date when this collection was last modified.
@ -34,7 +34,7 @@ namespace osu.Game.Collections
public BeatmapCollection() public BeatmapCollection()
{ {
Beatmaps.CollectionChanged += (_, __) => onChange(); BeatmapHashes.CollectionChanged += (_, __) => onChange();
Name.ValueChanged += _ => onChange(); Name.ValueChanged += _ => onChange();
} }

View File

@ -38,7 +38,7 @@ namespace osu.Game.Collections
} }
private readonly IBindableList<BeatmapCollection> collections = new BindableList<BeatmapCollection>(); private readonly IBindableList<BeatmapCollection> collections = new BindableList<BeatmapCollection>();
private readonly IBindableList<BeatmapInfo> beatmaps = new BindableList<BeatmapInfo>(); private readonly IBindableList<string> beatmaps = new BindableList<string>();
private readonly BindableList<CollectionFilterMenuItem> filters = new BindableList<CollectionFilterMenuItem>(); private readonly BindableList<CollectionFilterMenuItem> filters = new BindableList<CollectionFilterMenuItem>();
[Resolved(CanBeNull = true)] [Resolved(CanBeNull = true)]
@ -95,10 +95,10 @@ namespace osu.Game.Collections
beatmaps.CollectionChanged -= filterBeatmapsChanged; beatmaps.CollectionChanged -= filterBeatmapsChanged;
if (filter.OldValue?.Collection != null) if (filter.OldValue?.Collection != null)
beatmaps.UnbindFrom(filter.OldValue.Collection.Beatmaps); beatmaps.UnbindFrom(filter.OldValue.Collection.BeatmapHashes);
if (filter.NewValue?.Collection != null) if (filter.NewValue?.Collection != null)
beatmaps.BindTo(filter.NewValue.Collection.Beatmaps); beatmaps.BindTo(filter.NewValue.Collection.BeatmapHashes);
beatmaps.CollectionChanged += filterBeatmapsChanged; beatmaps.CollectionChanged += filterBeatmapsChanged;
@ -196,7 +196,7 @@ namespace osu.Game.Collections
private IBindable<WorkingBeatmap> beatmap { get; set; } private IBindable<WorkingBeatmap> beatmap { get; set; }
[CanBeNull] [CanBeNull]
private readonly BindableList<BeatmapInfo> collectionBeatmaps; private readonly BindableList<string> collectionBeatmaps;
[NotNull] [NotNull]
private readonly Bindable<string> collectionName; private readonly Bindable<string> collectionName;
@ -208,7 +208,7 @@ namespace osu.Game.Collections
public CollectionDropdownMenuItem(MenuItem item) public CollectionDropdownMenuItem(MenuItem item)
: base(item) : base(item)
{ {
collectionBeatmaps = Item.Collection?.Beatmaps.GetBoundCopy(); collectionBeatmaps = Item.Collection?.BeatmapHashes.GetBoundCopy();
collectionName = Item.CollectionName.GetBoundCopy(); collectionName = Item.CollectionName.GetBoundCopy();
} }
@ -258,7 +258,7 @@ namespace osu.Game.Collections
{ {
Debug.Assert(collectionBeatmaps != null); Debug.Assert(collectionBeatmaps != null);
beatmapInCollection = collectionBeatmaps.Contains(beatmap.Value.BeatmapInfo); beatmapInCollection = collectionBeatmaps.Contains(beatmap.Value.BeatmapInfo.MD5Hash);
addOrRemoveButton.Enabled.Value = !beatmap.IsDefault; addOrRemoveButton.Enabled.Value = !beatmap.IsDefault;
addOrRemoveButton.Icon = beatmapInCollection ? FontAwesome.Solid.MinusSquare : FontAwesome.Solid.PlusSquare; addOrRemoveButton.Icon = beatmapInCollection ? FontAwesome.Solid.MinusSquare : FontAwesome.Solid.PlusSquare;
@ -285,8 +285,8 @@ namespace osu.Game.Collections
{ {
Debug.Assert(collectionBeatmaps != null); Debug.Assert(collectionBeatmaps != null);
if (!collectionBeatmaps.Remove(beatmap.Value.BeatmapInfo)) if (!collectionBeatmaps.Remove(beatmap.Value.BeatmapInfo.MD5Hash))
collectionBeatmaps.Add(beatmap.Value.BeatmapInfo); collectionBeatmaps.Add(beatmap.Value.BeatmapInfo.MD5Hash);
} }
protected override Drawable CreateContent() => content = (Content)base.CreateContent(); protected override Drawable CreateContent() => content = (Content)base.CreateContent();

View File

@ -13,7 +13,6 @@ using osu.Framework.Bindables;
using osu.Framework.Graphics; using osu.Framework.Graphics;
using osu.Framework.Logging; using osu.Framework.Logging;
using osu.Framework.Platform; using osu.Framework.Platform;
using osu.Game.Beatmaps;
using osu.Game.Database; using osu.Game.Database;
using osu.Game.IO; using osu.Game.IO;
using osu.Game.IO.Legacy; using osu.Game.IO.Legacy;
@ -40,9 +39,6 @@ namespace osu.Game.Collections
public readonly BindableList<BeatmapCollection> Collections = new BindableList<BeatmapCollection>(); public readonly BindableList<BeatmapCollection> Collections = new BindableList<BeatmapCollection>();
[Resolved]
private BeatmapManager beatmaps { get; set; }
private readonly Storage storage; private readonly Storage storage;
public CollectionManager(Storage storage) public CollectionManager(Storage storage)
@ -173,10 +169,10 @@ namespace osu.Game.Collections
if (existing == null) if (existing == null)
Collections.Add(existing = new BeatmapCollection { Name = { Value = newCol.Name.Value } }); Collections.Add(existing = new BeatmapCollection { Name = { Value = newCol.Name.Value } });
foreach (var newBeatmap in newCol.Beatmaps) foreach (string newBeatmap in newCol.BeatmapHashes)
{ {
if (!existing.Beatmaps.Contains(newBeatmap)) if (!existing.BeatmapHashes.Contains(newBeatmap))
existing.Beatmaps.Add(newBeatmap); existing.BeatmapHashes.Add(newBeatmap);
} }
} }
@ -226,9 +222,7 @@ namespace osu.Game.Collections
string checksum = sr.ReadString(); string checksum = sr.ReadString();
var beatmap = beatmaps.QueryBeatmap(b => b.MD5Hash == checksum); collection.BeatmapHashes.Add(checksum);
if (beatmap != null)
collection.Beatmaps.Add(beatmap);
} }
if (notification != null) if (notification != null)
@ -299,11 +293,12 @@ namespace osu.Game.Collections
{ {
sw.Write(c.Name.Value); sw.Write(c.Name.Value);
var beatmapsCopy = c.Beatmaps.ToArray(); string[] beatmapsCopy = c.BeatmapHashes.ToArray();
sw.Write(beatmapsCopy.Length); sw.Write(beatmapsCopy.Length);
foreach (var b in beatmapsCopy) foreach (string b in beatmapsCopy)
sw.Write(b.MD5Hash); sw.Write(b);
} }
} }

View File

@ -13,7 +13,7 @@ namespace osu.Game.Collections
public DeleteCollectionDialog(BeatmapCollection collection, Action deleteAction) public DeleteCollectionDialog(BeatmapCollection collection, Action deleteAction)
{ {
HeaderText = "Confirm deletion of"; HeaderText = "Confirm deletion of";
BodyText = $"{collection.Name.Value} ({"beatmap".ToQuantity(collection.Beatmaps.Count)})"; BodyText = $"{collection.Name.Value} ({"beatmap".ToQuantity(collection.BeatmapHashes.Count)})";
Icon = FontAwesome.Regular.TrashAlt; Icon = FontAwesome.Regular.TrashAlt;

View File

@ -225,7 +225,7 @@ namespace osu.Game.Collections
{ {
background.FlashColour(Color4.White, 150); background.FlashColour(Color4.White, 150);
if (collection.Beatmaps.Count == 0) if (collection.BeatmapHashes.Count == 0)
deleteCollection(); deleteCollection();
else else
dialogOverlay?.Push(new DeleteCollectionDialog(collection, deleteCollection)); dialogOverlay?.Push(new DeleteCollectionDialog(collection, deleteCollection));

View File

@ -30,7 +30,15 @@ namespace osu.Game.Overlays.Music
var items = (SearchContainer<RearrangeableListItem<Live<BeatmapSetInfo>>>)ListContainer; var items = (SearchContainer<RearrangeableListItem<Live<BeatmapSetInfo>>>)ListContainer;
foreach (var item in items.OfType<PlaylistItem>()) foreach (var item in items.OfType<PlaylistItem>())
item.InSelectedCollection = criteria.Collection?.Beatmaps.Any(b => item.Model.ID == b.BeatmapSet?.ID) ?? true; {
if (criteria.Collection == null)
item.InSelectedCollection = true;
else
{
item.InSelectedCollection = item.Model.Value.Beatmaps.Select(b => b.MD5Hash)
.Any(criteria.Collection.BeatmapHashes.Contains);
}
}
items.SearchTerm = criteria.SearchText; items.SearchTerm = criteria.SearchText;
} }

View File

@ -72,7 +72,7 @@ namespace osu.Game.Screens.Select.Carousel
} }
if (match) if (match)
match &= criteria.Collection?.Beatmaps.Contains(BeatmapInfo) ?? true; match &= criteria.Collection?.BeatmapHashes.Contains(BeatmapInfo.MD5Hash) ?? true;
if (match && criteria.RulesetCriteria != null) if (match && criteria.RulesetCriteria != null)
match &= criteria.RulesetCriteria.Matches(BeatmapInfo); match &= criteria.RulesetCriteria.Matches(BeatmapInfo);

View File

@ -256,12 +256,12 @@ namespace osu.Game.Screens.Select.Carousel
return new ToggleMenuItem(collection.Name.Value, MenuItemType.Standard, s => return new ToggleMenuItem(collection.Name.Value, MenuItemType.Standard, s =>
{ {
if (s) if (s)
collection.Beatmaps.Add(beatmapInfo); collection.BeatmapHashes.Add(beatmapInfo.MD5Hash);
else else
collection.Beatmaps.Remove(beatmapInfo); collection.BeatmapHashes.Remove(beatmapInfo.MD5Hash);
}) })
{ {
State = { Value = collection.Beatmaps.Contains(beatmapInfo) } State = { Value = collection.BeatmapHashes.Contains(beatmapInfo.MD5Hash) }
}; };
} }

View File

@ -245,7 +245,7 @@ namespace osu.Game.Screens.Select.Carousel
TernaryState state; TernaryState state;
int countExisting = beatmapSet.Beatmaps.Count(b => collection.Beatmaps.Contains(b)); int countExisting = beatmapSet.Beatmaps.Count(b => collection.BeatmapHashes.Contains(b.MD5Hash));
if (countExisting == beatmapSet.Beatmaps.Count) if (countExisting == beatmapSet.Beatmaps.Count)
state = TernaryState.True; state = TernaryState.True;
@ -261,14 +261,14 @@ namespace osu.Game.Screens.Select.Carousel
switch (s) switch (s)
{ {
case TernaryState.True: case TernaryState.True:
if (collection.Beatmaps.Contains(b)) if (collection.BeatmapHashes.Contains(b.MD5Hash))
continue; continue;
collection.Beatmaps.Add(b); collection.BeatmapHashes.Add(b.MD5Hash);
break; break;
case TernaryState.False: case TernaryState.False:
collection.Beatmaps.Remove(b); collection.BeatmapHashes.Remove(b.MD5Hash);
break; break;
} }
} }