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 parsing of color strings into KnownColor enum, remove allocations #9669

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Aug 29, 2024

Description

Removes conversion of the color string into uppercase and its subsequent compare with StringComparison.OrdinalIgnoreCase.
The replacement was done by a simple regex.

Method colorString Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original SlateGray 18.853 ns 0.3373 ns 0.3155 ns 0.0024 6,066 B 40 B
PR__EDIT SlateGray 2.155 ns 0.0072 ns 0.0060 ns - 6,695 B -

Customer Impact

Increased performance on serialization, zero allocations.

Regression

No.

Testing

Local build/CI/assert test.

foreach (string color in Enum.GetNames<KnownColor>())
{
    KnownColor constantColor = Enum.Parse<KnownColor>(color);

    KnownColor lowerCase = ColorStringToKnownColorNEW(color.ToLowerInvariant());
    KnownColor upperCase = ColorStringToKnownColorNEW(color.ToUpperInvariant());
    KnownColor normalColor = ColorStringToKnownColorNEW(color);

    Assert.AreEqual<KnownColor>(constantColor, lowerCase);
    Assert.AreEqual<KnownColor>(constantColor, upperCase);
    Assert.AreEqual<KnownColor>(constantColor, normalColor);
}

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@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 29, 2024
@siagupta0202
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@siagupta0202
Copy link
Contributor

LGTM

@siagupta0202 siagupta0202 merged commit 793610e into dotnet:main Jan 23, 2025
8 checks passed
@siagupta0202
Copy link
Contributor

@h3xds1nz Thank you for your contributions!

@h3xds1nz
Copy link
Member Author

@siagupta0202 Thank you :)

h3xds1nz added a commit to h3xds1nz/wpf that referenced this pull request Jan 24, 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.

4 participants