dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.08k stars 1.17k forks source link

System.Windows.Input.ModifierKeysConverter.GetModifierKeys has unnecessary allocations #7502

Open paulomorgado opened 1 year ago

paulomorgado commented 1 year ago

This benchmark shows a possible improvement.

[MemoryDiagnoser]
public class ModifierKeysConverterBenchmarks
{
    private const char Modifier_Delimiter = '+';

    [Params("Control", "Control+Shift", "Control+Shift+Alt", "Control+Shift+Alt+Windows", " Control + Shift + Alt + Windows ")]
    public string Text;

    [Benchmark(Baseline = true)]
    public ModifierKeys GetModifierKeys() => GetModifierKeys(this.Text, CultureInfo.CurrentCulture);

    private ModifierKeys GetModifierKeys(string modifiersToken, CultureInfo culture)
    {
        ModifierKeys modifiers = ModifierKeys.None;
        if (modifiersToken.Length != 0)
        {
            int offset = 0;
            do
            {
                offset = modifiersToken.IndexOf(Modifier_Delimiter);
                string token = (offset < 0) ? modifiersToken : modifiersToken.Substring(0, offset);
                token = token.Trim();
                token = token.ToUpper(culture);

                if (token == String.Empty)
                    break;

                switch (token)
                {
                    case "CONTROL":
                    case "CTRL":
                        modifiers |= ModifierKeys.Control;
                        break;

                    case "SHIFT":
                        modifiers |= ModifierKeys.Shift;
                        break;

                    case "ALT":
                        modifiers |= ModifierKeys.Alt;
                        break;

                    case "WINDOWS":
                    case "WIN":
                        modifiers |= ModifierKeys.Windows;
                        break;

                    default:
                        throw new NotSupportedException();
                }

                modifiersToken = modifiersToken.Substring(offset + 1);
            } while (offset != -1);
        }
        return modifiers;
    }

    [Benchmark()]
    public ModifierKeys GetModifierKeysWithSpanSplit() => GetModifierKeysWithSpanSplit(this.Text);

    private ModifierKeys GetModifierKeysWithSpanSplit(string modifiersToken)
    {
        ModifierKeys modifiers = ModifierKeys.None;
        var modifiersTokenSpan = modifiersToken.AsSpan();
        if (modifiersTokenSpan.Length != 0)
        {
            var offset = 0;
            do
            {
                offset = modifiersTokenSpan.IndexOf(Modifier_Delimiter);
                var token = (offset < 0) ? modifiersTokenSpan : modifiersTokenSpan.Slice(0, offset);
                token = token.Trim();

                if (token.Length == 0)
                    break;

                if (token.Equals("CONTROL", StringComparison.OrdinalIgnoreCase)
                    || token.Equals("CTRL", StringComparison.OrdinalIgnoreCase))
                {
                    modifiers |= ModifierKeys.Control;
                }
                else if (token.Equals("SHIFT", StringComparison.OrdinalIgnoreCase))
                {
                    modifiers |= ModifierKeys.Shift;
                }
                else if (token.Equals("ALT", StringComparison.OrdinalIgnoreCase))
                {
                    modifiers |= ModifierKeys.Alt;
                }
                else if (token.Equals("WINDOWS", StringComparison.OrdinalIgnoreCase)
                    || token.Equals("WIN", StringComparison.OrdinalIgnoreCase))
                {
                    modifiers |= ModifierKeys.Windows;
                }
                else
                {
                    throw new NotSupportedException();
                }

                modifiersTokenSpan = modifiersTokenSpan.Slice(offset + 1);
            } while (offset != -1);
        }
        return modifiers;
    }
}
Method Text Mean Error StdDev Ratio Gen0 Allocated Alloc Ratio
GetModifierKeys Control 26.576 ns 0.5546 ns 0.4916 ns 1.00 0.0032 40 B 1.00
GetModifierKeysWithSpanSplit Control 8.825 ns 0.1766 ns 0.1475 ns 0.33 - - 0.00
GetModifierKeys Control+Shift 58.830 ns 0.5157 ns 0.4571 ns 1.00 0.0114 144 B 1.00
GetModifierKeysWithSpanSplit Control+Shift 16.081 ns 0.3394 ns 0.4758 ns 0.27 - - 0.00
GetModifierKeys Control+Shift+Alt 97.412 ns 1.8411 ns 1.4374 ns 1.00 0.0197 248 B 1.00
GetModifierKeysWithSpanSplit Control+Shift+Alt 23.919 ns 0.3381 ns 0.2998 ns 0.25 - - 0.00
GetModifierKeys Contr(...)ndows [25] 141.254 ns 2.7635 ns 3.5933 ns 1.00 0.0312 392 B 1.00
GetModifierKeysWithSpanSplit Contr(...)ndows [25] 33.901 ns 0.3016 ns 0.2674 ns 0.24 - - 0.00
GetModifierKeys Cont(...)dows [33] 175.144 ns 2.9857 ns 3.3186 ns 1.00 0.0451 568 B 1.00
GetModifierKeysWithSpanSplit Cont(...)dows [33] 43.114 ns 0.8671 ns 1.1274 ns 0.25 - - 0.00
miloush commented 1 year ago

@paulomorgado are you intending a PR?

paulomorgado commented 1 year ago

I might. Is this covered with unit tests?