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

Optimize conversion of KeyGesture from/to string, reduce allocations #10262

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Jan 11, 2025

Description

Parsing optimization for KeyGesture and its conversion from/to string. Also adds documentation for whole class.

Including both .NET 9/main vs PR benchmarks, to show improvements in #9683 / #9697 in perspective.

ConvertFrom Benchmarks (.NET 9 vs PR)

Method Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original 299.15 ns 4.620 ns 4.322 ns 0.0439 93 B 736 B
PR__EDIT 57.63 ns 0.533 ns 0.499 ns 0.0062 93 B 104 B
Original 193.40 ns 2.449 ns 2.171 ns 0.0157 93 B 264 B
PR__EDIT 30.03 ns 0.635 ns 0.679 ns 0.0062 93 B 104 B
Original 147.75 ns 1.653 ns 1.546 ns 0.0091 93 B 152 B
PR__EDIT 20.74 ns 0.122 ns 0.114 ns 0.0019 93 B 32 B

ConvertFrom Benchmarks (main vs PR)

Method Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original 106.77 ns 2.063 ns 2.755 ns 93 B 0.0257 432 B
PR__EDIT 57.50 ns 0.813 ns 0.721 ns 93 B 0.0062 104 B
Original 50.17 ns 0.870 ns 0.814 ns 93 B 0.0124 208 B
PR__EDIT 30.08 ns 0.610 ns 0.571 ns 93 B 0.0062 104 B
Original 32.98 ns 0.514 ns 0.456 ns 0.0057 93 B 96 B
PR__EDIT 20.94 ns 0.170 ns 0.151 ns 0.0019 93 B 32 B
[Arguments(" Ctrl + Shift + F10, This is a Test Gesture ")]
[Arguments("Ctrl+F10,This is a Test Gesture")]
[Arguments("Ctrl+F10")]
public KeyGesture Original(object source)
{
    return (KeyGesture)keyGestureConverter.ConvertFrom(null, null, source);
}

ConvertTo (.NET 9 vs PR) Benchmarks

Method Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original 66.92 ns 1.322 ns 1.522 ns 0.0210 107 B 352 B
PR__EDIT 32.34 ns 0.671 ns 0.628 ns 0.0072 107 B 120 B
Original 35.69 ns 0.745 ns 0.887 ns 0.0091 107 B 152 B
PR__EDIT 15.97 ns 0.359 ns 0.428 ns 0.0062 107 B 104 B
Original 35.27 ns 0.698 ns 0.618 ns 0.0086 107 B 144 B
PR__EDIT 21.76 ns 0.290 ns 0.257 ns 0.0038 107 B 64 B
Original 22.17 ns 0.359 ns 0.318 ns 0.0043 107 B 72 B
PR__EDIT 7.71 ns 0.140 ns 0.131 ns 0.0014 107 B 24 B

ConvertTo (main vs PR) Benchmarks

Method Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original 44.108 ns 0.9124 ns 1.5738 ns 0.0176 107 B 296 B
PR__EDIT 32.482 ns 0.7140 ns 0.7333 ns 0.0072 107 B 120 B
Original 20.743 ns 0.4282 ns 0.3796 ns 0.0076 107 B 128 B
PR__EDIT 16.058 ns 0.3185 ns 0.2823 ns 0.0062 107 B 104 B
Original 24.343 ns 0.5292 ns 0.6094 ns 0.0072 107 B 120 B
PR__EDIT 22.066 ns 0.2735 ns 0.2558 ns 0.0038 107 B 64 B
Original 12.664 ns 0.1591 ns 0.1329 ns 0.0029 107 B 48 B
PR__EDIT 7.950 ns 0.1524 ns 0.1426 ns 0.0014 107 B 24 B
yield return new KeyGesture(Key.F10, ModifierKeys.Control | ModifierKeys.Shift, "This is a Test Gesture");
yield return new KeyGesture(Key.F10, ModifierKeys.None, "This is a Test Gesture");
yield return new KeyGesture(Key.F10, ModifierKeys.Control);
yield return new KeyGesture(Key.F10, ModifierKeys.None);

public KeyGesture Original(object source)
{
    return (string)keyGestureConverter.ConvertTo(null, null, source, typeof(string));
}

Customer Impact

Increased performance, decreased allocations, less static memory.

Regression

No.

Testing

Local build, backed with #10259.

Risk

Low, changes have been tested extensively.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners January 11, 2025 14:38
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant