Timed Difficulty Attributes calculation optimization

[Givikap120]

Timed Difficulty Attributes used for pp counter, and it's very slow on long maps because of O(n^2 * logn) time complexity

Test on full Because Maybe mapset (https://osu.ppy.sh/beatmapsets/773014#osu/2151273). Out of 152 seconds of main function call:

• 68 seconds on OsuStrainSkill.DifficultyValue()
• 52 seconds on IBeatmap.GetMaxCombo()
• 29 seconds on IBeatmap.HitObjects.Count(h => h is HitObject)
• 3 seconds on Speed.RelevantNoteCount()

Out of 68 seconds of OsuStrainSkill.DifficultyValue():

• 48 seconds on initial sort
• 17 seconds on second sort (after diffspike nerf)

After the optimization - out of 13 seconds (-91%) of main function call:

• 10 seconds (-85%) on OsuStrainSkill.DifficultyValue()
• 0.06 seconds (-99.9%) on IBeatmap.GetMaxCombo()
• 0.07 seconds (-99.8%) ( on IBeatmap.HitObjects.Count(h => h is HitObject)
• 3 seconds (not touched) on Speed.RelevantNoteCount()

Out of 10 seconds of OsuStrainSkill.DifficultyValue():

• 6 seconds (-87.5%) on initial sort, 5 seconds of which is creating a copy, and only 1 second is actual sorting. Considering this - speedup on actual sorting is 43x (-97.7%)
• 0.5 seconds (-97%) on the second sort (after diffspike nerf)

Optimization consists of two parts:
First part makes Max Combo and Objects Counts to be calculated progressively in the ProgressiveBeatmap beatmap class instead of from zero each time, reducing time complexity from O(n^2) to O(n).

Second part is all about .DifficultyValue():
After the call of DifficultyValue() the sorted strains are saved in the Skill class. On the next call of DifficultyValue one of 4 things can happen:

1. There are no saved strains -> calculate normally
2. There are saved strains, but new ones were added since last save -> insert each strain in the sorted array
3. There are no new strains since save, but the last strain was changed -> replace old last strain with new one
4. No new strains and last strain is unchanged -> just use the saved strains
   Those steps are reducing sort time complexity from O(n^2 * logn) to O(nlogn). Copying time complexity remains O(n^2), making it the slowest part of the calculation now.

The second sort after diffspike nerf is also changed. Unnecessary copy was removed and sorting is done via insertions now, decreasing O(n^2) for copy and O(n^2 * logn) for sort to just O(nlogn) for insertion.

Before:

osu.2024-08-17.16-19-52-155.1.mp4

After:

osu.2024-08-17.16-45-28-019.1.mp4

The other gamemodes aren't touched directly, but as they all use StrainSkill - they will be faster as well. Taiko needs additional work, as it's still have TaikoDifficultyCalculator.combinedDifficultyValue with O(n^2 * logn) time complexity.

[bdach]

Before I go through all of the changes in this file (which, is a lot of code added, just for this use case) - why are things like this being reinvented here? Can you not just use SortedList or something?

I have a feeling that if you try SortedList or another proper data structure a lot of the code above might just disappear.

[Givikap120]

Using normal List I have full control over what and when is being sorted. SortedList is a dictionary with unique keys, what means that it's unsuitable for this task, as strains can have the same value. Some time ago I tried to do it with SortedSet (what still have the same "uniqueness" issue) and it was slower than normal way.

@tsunyoku probably can give better explanation on this as we discussed this problem with him

[bdach]

Also, it looks like Extension Method GetMaxCombo is only correct for STD and maybe for Taiko.

Huh? Care to elaborate? It looks correct to me.

[bdach]

Mania is definitely incorrect because of Long Notes giving combo without judgements

mania is correct for lazer mechanics. "long notes giving combo without judgements" is a stable thing we didn't reimplement.

[bdach]

It already does though?

osu/osu.Game.Rulesets.Mania/Difficulty/ManiaDifficultyCalculator.cs

Lines 62 to 68 in ebff2eb

	private static int maxComboForObject(HitObject hitObject)
	{
	if (hitObject is HoldNote hold)
	return 1 + (int)((hold.EndTime - hold.StartTime) / 100);
	return 1;
	}

[bdach]

That may well be true but that entire conversation is out of scope of this pull which is supposed to be an "optimisation". I'd advise to move it elsewhere.

[Givikap120]

That may well be true but that entire conversation is out of scope of this pull which is supposed to be an "optimisation". I'd advise to move it elsewhere.

okay I agree
then I'm just waiting on review