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 MouseAction enum from/to string, remove allocations #9676

Merged
merged 7 commits into from
Oct 1, 2024

Conversation

h3xds1nz
Copy link
Contributor

@h3xds1nz h3xds1nz commented Aug 30, 2024

Description

Parsing optimization for MouseAction and its conversion from/to string. Allocation-free.

  • Removes trimming on string, now trims on the span, resulting in no allocations.
  • Removes conversion ToUpperInvariant and uses StringComparison.OrdinalIgnoreCase instead.
  • This will allow for string interning between Convert/ConvertFrom and other literals, saving static memory.
  • The JITTed codegen takes up less memory as we removed few things.
  • Adds/completes code documentation for the whole class.

Last value in the enumeration

Method source Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size Allocated
Original MiddleDoubleClick 19.632 ns 0.2160 ns 0.2020 ns 0.0033 1,270 B 56 B
PR__EDIT MiddleDoubleClick 2.776 ns 0.0166 ns 0.0155 ns - 988 B -

Last value in the enumeration with space (trimmed)

Method source Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size Allocated
Original MiddleDoubleClick\x20 28.230 ns 0.5535 ns 0.5177 ns 0.0067 2,176 B 112 B
PR__EDIT MiddleDoubleClick\x20 5.659 ns 0.0315 ns 0.0280 ns - 1,448 B -

Customer Impact

Increased performance, zero allocations, less static memory.

Regression

No.

Testing

Local build, CI, assert test:

//Special case
string emptyToNone = string.Empty;
MouseAction constantName = MouseAction.None;

MouseAction lowerCase = (MouseAction)ConvertFrom(null, null, emptyToNone.ToLowerInvariant());
MouseAction upperCase = (MouseAction)ConvertFrom(null, null, emptyToNone.ToUpperInvariant());
MouseAction normalColor = (MouseAction)ConvertFrom(null, null, emptyToNone);

Assert.AreEqual<MouseAction>(constantName, lowerCase);
Assert.AreEqual<MouseAction>(constantName, upperCase);
Assert.AreEqual<MouseAction>(constantName, normalColor);

foreach (string action in Enum.GetNames<MouseAction>())
{
     constantName = Enum.Parse<MouseAction>(action);

     lowerCase = (MouseAction)ConvertFrom(null, null, action.ToLowerInvariant());
     upperCase = (MouseAction)ConvertFrom(null, null, action.ToUpperInvariant());
     normalColor = (MouseAction)ConvertFrom(null, null, action);

    Assert.AreEqual<MouseAction>(constantName, lowerCase);
    Assert.AreEqual<MouseAction>(constantName, upperCase);
    Assert.AreEqual<MouseAction>(constantName, normalColor);

    //Back to the original values
    string result = (string)ConvertTo(null, null, constantName, typeof(string));
    result = result == string.Empty ? "None" : result; //Special case
    Assert.AreEqual(action, result);
}

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners August 30, 2024 12:43
@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 Aug 30, 2024
@harshit7962
Copy link
Member

@h3xds1nz Thank you for contributing.

@harshit7962 harshit7962 merged commit d01dfc9 into dotnet:main Oct 1, 2024
8 checks passed
@h3xds1nz
Copy link
Contributor Author

h3xds1nz commented Oct 1, 2024

@harshit7962 Happy to see this one get in, thank you :)

@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

3 participants