diff --git a/osu.Game.Tests/Beatmaps/BeatmapDifficultyManagerTest.cs b/osu.Game.Tests/Beatmaps/BeatmapDifficultyManagerTest.cs index ec77f48063..70503bec7a 100644 --- a/osu.Game.Tests/Beatmaps/BeatmapDifficultyManagerTest.cs +++ b/osu.Game.Tests/Beatmaps/BeatmapDifficultyManagerTest.cs @@ -3,6 +3,7 @@ using NUnit.Framework; using osu.Game.Beatmaps; +using osu.Game.Rulesets; using osu.Game.Rulesets.Mods; using osu.Game.Rulesets.Osu.Mods; @@ -14,8 +15,8 @@ public class BeatmapDifficultyManagerTest [Test] public void TestKeyEqualsWithDifferentModInstances() { - var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHardRock(), new OsuModHidden() }); - var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHardRock(), new OsuModHidden() }); + var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHardRock(), new OsuModHidden() }); + var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHardRock(), new OsuModHidden() }); Assert.That(key1, Is.EqualTo(key2)); } @@ -23,8 +24,8 @@ public void TestKeyEqualsWithDifferentModInstances() [Test] public void TestKeyEqualsWithDifferentModOrder() { - var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHardRock(), new OsuModHidden() }); - var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(1234, 0, new Mod[] { new OsuModHidden(), new OsuModHardRock() }); + var key1 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHardRock(), new OsuModHidden() }); + var key2 = new BeatmapDifficultyCache.DifficultyCacheLookup(new BeatmapInfo { ID = 1234 }, new RulesetInfo { ID = 0 }, new Mod[] { new OsuModHidden(), new OsuModHardRock() }); Assert.That(key1, Is.EqualTo(key2)); } diff --git a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs index af1b1de0c1..126e08a173 100644 --- a/osu.Game/Beatmaps/BeatmapDifficultyCache.cs +++ b/osu.Game/Beatmaps/BeatmapDifficultyCache.cs @@ -86,20 +86,30 @@ public IBindable GetBindableDifficulty([NotNull] BeatmapInfo bea /// The s to get the difficulty with. /// An optional which stops computing the star difficulty. /// The . - public async Task GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable mods = null, - CancellationToken cancellationToken = default) + public Task GetDifficultyAsync([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable mods = null, CancellationToken cancellationToken = default) { - if (tryGetExisting(beatmapInfo, rulesetInfo, mods, out var existing, out var key)) - return existing; + // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset. + rulesetInfo ??= beatmapInfo.Ruleset; - return await Task.Factory.StartNew(() => + // Difficulty can only be computed if the beatmap and ruleset are locally available. + if (beatmapInfo.ID == 0 || rulesetInfo.ID == null) { - // Computation may have finished in a previous task. - if (tryGetExisting(beatmapInfo, rulesetInfo, mods, out existing, out _)) + // If not, fall back to the existing star difficulty (e.g. from an online source). + return Task.FromResult(new StarDifficulty(beatmapInfo.StarDifficulty, beatmapInfo.MaxCombo ?? 0)); + } + + return GetAsync(new DifficultyCacheLookup(beatmapInfo, rulesetInfo, mods), cancellationToken); + } + + protected override Task ComputeValueAsync(DifficultyCacheLookup lookup, CancellationToken token = default) + { + return Task.Factory.StartNew(() => + { + if (CheckExists(lookup, out var existing)) return existing; - return computeDifficulty(key, beatmapInfo, rulesetInfo); - }, cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); + return computeDifficulty(lookup); + }, token, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler); } /// @@ -111,10 +121,8 @@ public async Task GetDifficultyAsync([NotNull] BeatmapInfo beatm /// The . public StarDifficulty GetDifficulty([NotNull] BeatmapInfo beatmapInfo, [CanBeNull] RulesetInfo rulesetInfo = null, [CanBeNull] IEnumerable mods = null) { - if (tryGetExisting(beatmapInfo, rulesetInfo, mods, out var existing, out var key)) - return existing; - - return computeDifficulty(key, beatmapInfo, rulesetInfo); + // this is safe in this usage because the only asynchronous part is handled by the local scheduler. + return GetDifficultyAsync(beatmapInfo, rulesetInfo, mods).Result; } /// @@ -207,38 +215,38 @@ private BindableStarDifficulty createBindable([NotNull] BeatmapInfo beatmapInfo, /// A token that may be used to cancel this update. private void updateBindable([NotNull] BindableStarDifficulty bindable, [CanBeNull] RulesetInfo rulesetInfo, [CanBeNull] IEnumerable mods, CancellationToken cancellationToken = default) { - GetDifficultyAsync(bindable.Beatmap, rulesetInfo, mods, cancellationToken).ContinueWith(t => - { - // We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events. - Schedule(() => + GetAsync(new DifficultyCacheLookup(bindable.Beatmap, rulesetInfo, mods), cancellationToken) + .ContinueWith(t => { - if (!cancellationToken.IsCancellationRequested) - bindable.Value = t.Result; - }); - }, cancellationToken); + // We're on a threadpool thread, but we should exit back to the update thread so consumers can safely handle value-changed events. + Schedule(() => + { + if (!cancellationToken.IsCancellationRequested) + bindable.Value = t.Result; + }); + }, cancellationToken); } /// /// Computes the difficulty defined by a key, and stores it to the timed cache. /// /// The that defines the computation parameters. - /// The to compute the difficulty of. - /// The to compute the difficulty with. /// The . - private StarDifficulty computeDifficulty(in DifficultyCacheLookup key, BeatmapInfo beatmapInfo, RulesetInfo rulesetInfo) + private StarDifficulty computeDifficulty(in DifficultyCacheLookup key) { // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset. - rulesetInfo ??= beatmapInfo.Ruleset; + var beatmapInfo = key.Beatmap; + var rulesetInfo = key.Ruleset; try { var ruleset = rulesetInfo.CreateInstance(); Debug.Assert(ruleset != null); - var calculator = ruleset.CreateDifficultyCalculator(beatmapManager.GetWorkingBeatmap(beatmapInfo)); - var attributes = calculator.Calculate(key.Mods); + var calculator = ruleset.CreateDifficultyCalculator(beatmapManager.GetWorkingBeatmap(key.Beatmap)); + var attributes = calculator.Calculate(key.OrderedMods); - return Cache[key] = new StarDifficulty(attributes); + return new StarDifficulty(attributes); } catch (BeatmapInvalidForRulesetException e) { @@ -249,49 +257,17 @@ private StarDifficulty computeDifficulty(in DifficultyCacheLookup key, BeatmapIn if (rulesetInfo.Equals(beatmapInfo.Ruleset)) { Logger.Error(e, $"Failed to convert {beatmapInfo.OnlineBeatmapID} to the beatmap's default ruleset ({beatmapInfo.Ruleset})."); - return Cache[key] = new StarDifficulty(); + return new StarDifficulty(); } - // Check the cache first because this is now a different ruleset than the one previously guarded against. - if (tryGetExisting(beatmapInfo, beatmapInfo.Ruleset, Array.Empty(), out var existingDefault, out var existingDefaultKey)) - return existingDefault; - - return computeDifficulty(existingDefaultKey, beatmapInfo, beatmapInfo.Ruleset); + return GetAsync(new DifficultyCacheLookup(key.Beatmap, key.Beatmap.Ruleset, key.OrderedMods)).Result; } catch { - return Cache[key] = new StarDifficulty(); + return new StarDifficulty(); } } - /// - /// Attempts to retrieve an existing difficulty for the combination. - /// - /// The . - /// The . - /// The s. - /// The existing difficulty value, if present. - /// The key that was used to perform this lookup. This can be further used to query . - /// Whether an existing difficulty was found. - private bool tryGetExisting(BeatmapInfo beatmapInfo, RulesetInfo rulesetInfo, IEnumerable mods, out StarDifficulty existingDifficulty, out DifficultyCacheLookup key) - { - // In the case that the user hasn't given us a ruleset, use the beatmap's default ruleset. - rulesetInfo ??= beatmapInfo.Ruleset; - - // Difficulty can only be computed if the beatmap and ruleset are locally available. - if (beatmapInfo.ID == 0 || rulesetInfo.ID == null) - { - // If not, fall back to the existing star difficulty (e.g. from an online source). - existingDifficulty = new StarDifficulty(beatmapInfo.StarDifficulty, beatmapInfo.MaxCombo ?? 0); - key = default; - - return true; - } - - key = new DifficultyCacheLookup(beatmapInfo.ID, rulesetInfo.ID.Value, mods); - return Cache.TryGetValue(key, out existingDifficulty); - } - protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); @@ -302,29 +278,31 @@ protected override void Dispose(bool isDisposing) public readonly struct DifficultyCacheLookup : IEquatable { - public readonly int BeatmapId; - public readonly int RulesetId; - public readonly Mod[] Mods; + public readonly BeatmapInfo Beatmap; + public readonly RulesetInfo Ruleset; - public DifficultyCacheLookup(int beatmapId, int rulesetId, IEnumerable mods) + public readonly Mod[] OrderedMods; + + public DifficultyCacheLookup([NotNull] BeatmapInfo beatmap, [NotNull] RulesetInfo ruleset, IEnumerable mods) { - BeatmapId = beatmapId; - RulesetId = rulesetId; - Mods = mods?.OrderBy(m => m.Acronym).ToArray() ?? Array.Empty(); + Beatmap = beatmap; + Ruleset = ruleset; + OrderedMods = mods?.OrderBy(m => m.Acronym).ToArray() ?? Array.Empty(); } public bool Equals(DifficultyCacheLookup other) - => BeatmapId == other.BeatmapId - && RulesetId == other.RulesetId - && Mods.Select(m => m.Acronym).SequenceEqual(other.Mods.Select(m => m.Acronym)); + => Beatmap.ID == other.Beatmap.ID + && Ruleset.ID == other.Ruleset.ID + && OrderedMods.Select(m => m.Acronym).SequenceEqual(other.OrderedMods.Select(m => m.Acronym)); public override int GetHashCode() { var hashCode = new HashCode(); - hashCode.Add(BeatmapId); - hashCode.Add(RulesetId); - foreach (var mod in Mods) + hashCode.Add(Beatmap.ID); + hashCode.Add(Ruleset.ID); + + foreach (var mod in OrderedMods) hashCode.Add(mod.Acronym); return hashCode.ToHashCode(); diff --git a/osu.Game/Database/MemoryCachingComponent.cs b/osu.Game/Database/MemoryCachingComponent.cs index afdf37fa27..efc31f2b46 100644 --- a/osu.Game/Database/MemoryCachingComponent.cs +++ b/osu.Game/Database/MemoryCachingComponent.cs @@ -20,7 +20,7 @@ public abstract class MemoryCachingComponent : Component protected virtual bool CacheNullValues => true; /// - /// Retrieve the cached value for the given . + /// Retrieve the cached value for the given lookup. /// /// The lookup to retrieve. /// An optional to cancel the operation. @@ -37,6 +37,9 @@ protected async Task GetAsync([NotNull] TLookup lookup, CancellationToke return computed; } + protected bool CheckExists([NotNull] TLookup lookup, out TValue value) => + cache.TryGetValue(lookup, out value); + /// /// Called on cache miss to compute the value for the specified lookup. ///