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

Fix huge allocation overhead in UnstableRateCounter #26976

Merged
merged 6 commits into from Feb 5, 2024

Conversation

EVAST9919
Copy link
Contributor

Overview

Currently UnstableRateCounter is allocationg a new array of size of judged objects on each judgement, which quickly becoming a problem for maps with many objects. By using Welford's online algorithm we can avoid selection and multiple passes over hitEvents and compute UR in a single loop.

master pr
master pr

Benchmark

Method Mean Error StdDev Gen0 Allocated
BenchmarkOldUR 62.75 us 0.079 us 0.066 us 8.0566 16968 B
BenchmarkNewUR 29.24 us 0.033 us 0.027 us 0.0305 96 B
Code
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System.Collections.Generic;
using BenchmarkDotNet.Attributes;
using osu.Framework.Utils;
using osu.Game.Rulesets.Objects;
using osu.Game.Rulesets.Scoring;

namespace osu.Game.Benchmarks
{
    public class BenchmarkUR : BenchmarkTest
    {
        private List<HitEvent> events = null!;

        public override void SetUp()
        {
            base.SetUp();
            events = new List<HitEvent>();

            for (int i = 0; i < 1000; i++)
                events.Add(new HitEvent(RNG.NextDouble(-200.0, 200.0), RNG.NextDouble(1.0, 2.0), HitResult.Great, new HitObject(), null, null));
        }

        [Benchmark]
        public void BenchmarkOldUR()
        {
            events.CalculateUnstableRateOld();
        }

        [Benchmark]
        public void BenchmarkNewUR()
        {
            events.CalculateUnstableRate();
        }
    }
}

Algorithm comparison

There is a UnstableRateTest test scene but just in case I compared old and new algo manually, results are basically the same:
diff

@bdach
Copy link
Collaborator

bdach commented Feb 5, 2024

Benchmark results are a bit more interesting here but not changed that much.

Method Mean Error StdDev Gen0 Allocated
CalculateUnstableRate (old) 27.08 us 0.418 us 0.890 us 1.3428 16.54 KB
CalculateUnstableRate (new) 37.06 us 2.236 us 6.593 us 42.14 us 96 B

bdach and others added 2 commits February 5, 2024 19:10
Co-authored-by: Andrei Zavatski <megaman9919@gmail.com>
@bdach bdach enabled auto-merge February 5, 2024 18:15
@bdach bdach merged commit 4a6d392 into ppy:master Feb 5, 2024
10 of 11 checks passed
@EVAST9919 EVAST9919 deleted the ur-alloc branch February 5, 2024 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants