From b530d02a70115f6c550813379dc9c38b1a8ec76c Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 11 Jan 2024 13:54:25 +0900 Subject: [PATCH 1/2] Remove allocations during bass device sync polling operation --- .../Audio/AudioManagerWithDeviceLoss.cs | 16 +++-- osu.Framework/Audio/AudioManager.cs | 67 +++++++++++++------ 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/osu.Framework.Tests/Audio/AudioManagerWithDeviceLoss.cs b/osu.Framework.Tests/Audio/AudioManagerWithDeviceLoss.cs index d1f5977b35..311468c0b6 100644 --- a/osu.Framework.Tests/Audio/AudioManagerWithDeviceLoss.cs +++ b/osu.Framework.Tests/Audio/AudioManagerWithDeviceLoss.cs @@ -1,7 +1,7 @@ // Copyright (c) ppy Pty Ltd . Licensed under the MIT Licence. // See the LICENCE file in the repository root for full licence text. -using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using ManagedBass; using osu.Framework.Audio; @@ -40,12 +40,20 @@ protected override bool InitBass(int device) } } - protected override IEnumerable EnumerateAllDevices() + protected override bool CheckForDeviceChanges(ImmutableArray previousDevices) { - var devices = base.EnumerateAllDevices(); + if (simulateLoss) + return true; + + return base.CheckForDeviceChanges(previousDevices); + } + + protected override ImmutableArray GetAllDevices() + { + var devices = base.GetAllDevices(); if (simulateLoss) - devices = devices.Take(BASS_INTERNAL_DEVICE_COUNT); + devices = devices.Take(BASS_INTERNAL_DEVICE_COUNT).ToImmutableArray(); return devices; } diff --git a/osu.Framework/Audio/AudioManager.cs b/osu.Framework/Audio/AudioManager.cs index f7fe849fee..f6c366271d 100644 --- a/osu.Framework/Audio/AudioManager.cs +++ b/osu.Framework/Audio/AudioManager.cs @@ -137,7 +137,7 @@ public class AudioManager : AudioCollectionManager Bass.CurrentDevice != Bass.DefaultDevice; // Mutated by multiple threads, must be thread safe. - private ImmutableList audioDevices = ImmutableList.Empty; + private ImmutableArray audioDevices = ImmutableArray.Empty; private ImmutableList audioDeviceNames = ImmutableList.Empty; private Scheduler scheduler => thread.Scheduler; @@ -145,7 +145,6 @@ public class AudioManager : AudioCollectionManager private Scheduler eventScheduler => EventScheduler ?? scheduler; private readonly CancellationTokenSource cancelSource = new CancellationTokenSource(); - private readonly DeviceInfoUpdateComparer updateComparer = new DeviceInfoUpdateComparer(); /// /// The scheduler used for invoking publicly exposed delegate events. @@ -208,7 +207,8 @@ public AudioManager(AudioThread audioThread, ResourceStore trackStore, R { try { - syncAudioDevices(); + if (CheckForDeviceChanges(audioDevices)) + syncAudioDevices(); Thread.Sleep(1000); } catch @@ -427,17 +427,10 @@ protected virtual bool InitBass(int device) private void syncAudioDevices() { - // audioDevices are updated if: - // - A new device is added - // - An existing device is Enabled/Disabled or set as Default - var updatedAudioDevices = EnumerateAllDevices().ToImmutableList(); - if (audioDevices.SequenceEqual(updatedAudioDevices, updateComparer)) - return; - - audioDevices = updatedAudioDevices; + audioDevices = GetAllDevices(); // Bass should always be providing "No sound" and "Default" device. - Trace.Assert(audioDevices.Count >= BASS_INTERNAL_DEVICE_COUNT, "Bass did not provide any audio devices."); + Trace.Assert(audioDevices.Length >= BASS_INTERNAL_DEVICE_COUNT, "Bass did not provide any audio devices."); var oldDeviceNames = audioDeviceNames; var newDeviceNames = audioDeviceNames = audioDevices.Skip(BASS_INTERNAL_DEVICE_COUNT).Where(d => d.IsEnabled).Select(d => d.Name).ToImmutableList(); @@ -459,11 +452,50 @@ private void syncAudioDevices() } } - protected virtual IEnumerable EnumerateAllDevices() + /// + /// Check whether any audio device changes have occurred. + /// + /// Changes supported are: + /// - A new device is added + /// - An existing device is Enabled/Disabled or set as Default + /// + /// + /// This method is optimised to incur the lowest overhead possible. + /// + /// The previous audio devices array. + /// Whether a change was detected. + protected virtual bool CheckForDeviceChanges(ImmutableArray previousDevices) { int deviceCount = Bass.DeviceCount; + + if (previousDevices.Length != deviceCount) + return true; + for (int i = 0; i < deviceCount; i++) - yield return Bass.GetDeviceInfo(i); + { + var prevInfo = previousDevices[i]; + + Bass.GetDeviceInfo(i, out var info); + + if (info.IsEnabled != prevInfo.IsEnabled) + return true; + + if (info.IsDefault != prevInfo.IsDefault) + return true; + } + + return false; + } + + protected virtual ImmutableArray GetAllDevices() + { + int deviceCount = Bass.DeviceCount; + + var devices = ImmutableArray.CreateBuilder(deviceCount); + for (int i = 0; i < deviceCount; i++) + devices.Add(Bass.GetDeviceInfo(i)); + + return devices.MoveToImmutable(); } // The current device is considered valid if it is enabled, initialized, and not a fallback device. @@ -479,12 +511,5 @@ public override string ToString() string deviceName = audioDevices.ElementAtOrDefault(Bass.CurrentDevice).Name; return $@"{GetType().ReadableName()} ({deviceName ?? "Unknown"})"; } - - private class DeviceInfoUpdateComparer : IEqualityComparer - { - public bool Equals(DeviceInfo x, DeviceInfo y) => x.IsEnabled == y.IsEnabled && x.IsDefault == y.IsDefault; - - public int GetHashCode(DeviceInfo obj) => obj.Name.GetHashCode(); - } } } From 7ce8d4749880a32167a5670b39e8184b40fb8570 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 11 Jan 2024 14:14:41 +0900 Subject: [PATCH 2/2] Fix incorrect calls on `ImmutableArray.Builder` causing extra allocations --- osu.Framework/Input/Bindings/KeyCombination.cs | 4 +++- osu.Framework/Platform/SDL2Window_Windowing.cs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/osu.Framework/Input/Bindings/KeyCombination.cs b/osu.Framework/Input/Bindings/KeyCombination.cs index a412c3be8b..395b109b10 100644 --- a/osu.Framework/Input/Bindings/KeyCombination.cs +++ b/osu.Framework/Input/Bindings/KeyCombination.cs @@ -644,7 +644,9 @@ public static KeyCombination FromInputState(InputState state, Vector2? scrollDel Debug.Assert(!keys.Contains(InputKey.None)); // Having None in pressed keys will break IsPressed keys.Sort(); - return new KeyCombination(keys.ToImmutable()); + + // Can't use `MoveToImmutable` here as we don't provide accurate capacity. + return new KeyCombination(keys.ToImmutableList()); } } diff --git a/osu.Framework/Platform/SDL2Window_Windowing.cs b/osu.Framework/Platform/SDL2Window_Windowing.cs index 6f884cfaf3..b9b0cdeeaf 100644 --- a/osu.Framework/Platform/SDL2Window_Windowing.cs +++ b/osu.Framework/Platform/SDL2Window_Windowing.cs @@ -341,7 +341,7 @@ private static IEnumerable getSDLDisplays() Logger.Log($"Failed to retrieve SDL display at index ({i})", level: LogLevel.Error); } - return builder.ToImmutable(); + return builder.MoveToImmutable(); } private static bool tryGetDisplayFromSDL(int displayIndex, [NotNullWhen(true)] out Display? display)