-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Remove KnownColors static constructor #9661
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
Remove KnownColors static constructor #9661
Conversation
src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Knowncolors.cs
Outdated
Show resolved
Hide resolved
Faster solution can be achieved by using But that ~10 kB per app, the creation time of the dictionary, vs. 2ns gained per compare on a function that's only called during serialization; this is clearly a better solution for the case, so yeah, once again, well done, Thomas. |
Yes I initially had the same thought but while working on it I found that, when comparing my current changes vs doing a more optimized dictionary lookup, my current changes were cleaner, faster (I didn't see the 2ns gain you mention) and used less memory . But as you said, even if there was a 2ns gain by doing a more optimized lookup I think we should go with my current changes since it's cleaner and it removes the need for a 10 Kb dictionary. |
67666ab
to
29c9b42
Compare
I've rebased on main to add a unit test that validates my changes. I also pushed a change to "fix" the method when |
59ac317
to
59604ea
Compare
I rebased on main to fix the conflicts. |
Thanks @ThomasGoulet73, good contribution. |
Description
Removes KnownColors static constructor and replaces it with uint parsing and calling Enum.IsDefined. The static constructor is called at the startup of every WPF app AFAIK (It is called even with an app created from the empty WPF template). The size of the dictionary at runtime is about 10 Kb so not much but it is still unnecessary used memory.
Benchmark results of KnownColors.ArgbStringToKnownColor
Customer Impact
Better perf, better startup perf and lower static memory.
Regression
No.
Testing
Local testing.
Risk
Low.
Microsoft Reviewers: Open in CodeFlow