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

Performance optimizations for SvgColourConverter parser #806

Closed

Conversation

wieslawsoltes
Copy link
Contributor

@wieslawsoltes wieslawsoltes commented Jan 14, 2021

Reference Issue

Split from #786
Fixes: #795

What does this implement/fix? Explain your changes.

Performance optimization for

  • SvgColourConverter
  • SvgPaintServerFactory.Create

Any other comments?

dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_html_4*'
|                              Method |     Mean |     Error |    StdDev |      Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|------------------------------------ |---------:|----------:|----------:|----------:|-----:|-------:|------:|------:|----------:|
|     SvgColourConverter_Parse_html_4 | 3.798 us | 0.0381 us | 0.0318 us | 263,323.0 |    1 |      - |     - |     - |         - |
| SvgColourConverter_Parse_html_4_OLD | 6.223 us | 0.0249 us | 0.0221 us | 160,697.8 |    2 | 0.1526 |     - |     - |     640 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_system_colors_single_*'
|                                                          Method |      Mean |    Error |   StdDev |         Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------------------------------------- |----------:|---------:|---------:|-------------:|-----:|-------:|------:|------:|----------:|
|      SvgColourConverter_Parse_system_colors_single_DIRECT_first |  99.84 ns | 0.593 ns | 0.555 ns | 10,016,091.1 |    5 |      - |     - |     - |         - |
|     SvgColourConverter_Parse_system_colors_single_DIRECT_middle | 105.12 ns | 0.205 ns | 0.380 ns |  9,512,809.1 |    6 |      - |     - |     - |         - |
|       SvgColourConverter_Parse_system_colors_single_DIRECT_last |  91.76 ns | 0.318 ns | 0.282 ns | 10,897,650.7 |    4 |      - |     - |     - |         - |
|             SvgColourConverter_Parse_system_colors_single_first | 369.97 ns | 0.891 ns | 0.744 ns |  2,702,943.5 |    9 |      - |     - |     - |         - |
|            SvgColourConverter_Parse_system_colors_single_middle | 341.07 ns | 0.891 ns | 0.695 ns |  2,931,972.5 |    8 |      - |     - |     - |         - |
|              SvgColourConverter_Parse_system_colors_single_last | 342.55 ns | 0.416 ns | 0.325 ns |  2,919,254.4 |    8 |      - |     - |     - |         - |
|  SvgColourConverter_Parse_system_colors_single_DIRECT_first_OLD |  58.42 ns | 0.261 ns | 0.244 ns | 17,117,744.8 |    2 | 0.0114 |     - |     - |      48 B |
| SvgColourConverter_Parse_system_colors_single_DIRECT_middle_OLD |  61.11 ns | 0.199 ns | 0.177 ns | 16,363,623.1 |    3 | 0.0134 |     - |     - |      56 B |
|   SvgColourConverter_Parse_system_colors_single_DIRECT_last_OLD |  56.49 ns | 0.235 ns | 0.208 ns | 17,701,747.9 |    1 | 0.0114 |     - |     - |      48 B |
|         SvgColourConverter_Parse_system_colors_single_first_OLD | 246.43 ns | 1.584 ns | 1.482 ns |  4,057,884.3 |    7 | 0.0210 |     - |     - |      88 B |
|        SvgColourConverter_Parse_system_colors_single_middle_OLD | 247.99 ns | 0.894 ns | 0.793 ns |  4,032,467.1 |    7 | 0.0229 |     - |     - |      96 B |
|          SvgColourConverter_Parse_system_colors_single_last_OLD | 243.23 ns | 0.999 ns | 0.834 ns |  4,111,389.3 |    7 | 0.0210 |     - |     - |      88 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_system_colors_single_DIRECT*'
|                                                          Method |      Mean |    Error |   StdDev |         Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------------------------------------- |----------:|---------:|---------:|-------------:|-----:|-------:|------:|------:|----------:|
|      SvgColourConverter_Parse_system_colors_single_DIRECT_first | 125.88 ns | 0.491 ns | 0.459 ns |  7,944,300.4 |    5 |      - |     - |     - |         - |
|     SvgColourConverter_Parse_system_colors_single_DIRECT_middle | 131.03 ns | 0.346 ns | 0.306 ns |  7,631,799.0 |    6 |      - |     - |     - |         - |
|       SvgColourConverter_Parse_system_colors_single_DIRECT_last |  91.35 ns | 0.148 ns | 0.123 ns | 10,947,448.2 |    4 |      - |     - |     - |         - |
|  SvgColourConverter_Parse_system_colors_single_DIRECT_first_OLD |  63.99 ns | 0.299 ns | 0.265 ns | 15,628,463.4 |    3 | 0.0114 |     - |     - |      48 B |
| SvgColourConverter_Parse_system_colors_single_DIRECT_middle_OLD |  60.95 ns | 0.105 ns | 0.093 ns | 16,408,200.7 |    2 | 0.0134 |     - |     - |      56 B |
|   SvgColourConverter_Parse_system_colors_single_DIRECT_last_OLD |  56.59 ns | 0.232 ns | 0.205 ns | 17,670,523.2 |    1 | 0.0114 |     - |     - |      48 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_hex_rgb*'
|                               Method |       Mean |    Error |   StdDev |        Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|------------------------------------- |-----------:|---------:|---------:|------------:|-----:|-------:|------:|------:|----------:|
|     SvgColourConverter_Parse_hex_rgb |   538.4 ns |  1.11 ns |  0.99 ns | 1,857,347.1 |    1 |      - |     - |     - |         - |
| SvgColourConverter_Parse_hex_rgb_OLD | 5,353.7 ns | 25.09 ns | 20.95 ns |   186,786.0 |    2 | 0.2365 |     - |     - |    1008 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_hex_rrggbb*'
|                                  Method |     Mean |     Error |    StdDev |      Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|---------------------------------------- |---------:|----------:|----------:|----------:|-----:|-------:|------:|------:|----------:|
|     SvgColourConverter_Parse_hex_rrggbb | 1.028 us | 0.0016 us | 0.0014 us | 973,219.8 |    1 |      - |     - |     - |         - |
| SvgColourConverter_Parse_hex_rrggbb_OLD | 7.443 us | 0.0143 us | 0.0120 us | 134,362.9 |    2 | 0.1144 |     - |     - |     480 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_rgb_integer_range*'
|                                         Method |     Mean |     Error |    StdDev |      Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|----------------------------------------------- |---------:|----------:|----------:|----------:|-----:|-------:|------:|------:|----------:|
|     SvgColourConverter_Parse_rgb_integer_range | 1.316 us | 0.0031 us | 0.0029 us | 760,141.3 |    1 |      - |     - |     - |         - |
| SvgColourConverter_Parse_rgb_integer_range_OLD | 2.401 us | 0.0061 us | 0.0054 us | 416,552.0 |    2 | 0.2861 |     - |     - |    1208 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_rgb_float_range*'
|                                       Method |     Mean |     Error |    StdDev |      Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------------------------------------- |---------:|----------:|----------:|----------:|-----:|-------:|------:|------:|----------:|
|     SvgColourConverter_Parse_rgb_float_range | 2.386 us | 0.0052 us | 0.0044 us | 419,074.2 |    1 |      - |     - |     - |         - |
| SvgColourConverter_Parse_rgb_float_range_OLD | 3.833 us | 0.0146 us | 0.0136 us | 260,877.1 |    2 | 0.4883 |     - |     - |    2056 B |
dotnet run -c Release -f netcoreapp3.1 -- -f '*SvgColourConverter_Parse_rgb_hsl*'
|                               Method |     Mean |     Error |    StdDev |      Op/s | Rank |  Gen 0 | Gen 1 | Gen 2 | Allocated |
|------------------------------------- |---------:|----------:|----------:|----------:|-----:|-------:|------:|------:|----------:|
|     SvgColourConverter_Parse_rgb_hsl | 3.553 us | 0.0073 us | 0.0065 us | 281,458.3 |    1 |      - |     - |     - |         - |
| SvgColourConverter_Parse_rgb_hsl_OLD | 5.611 us | 0.0124 us | 0.0097 us | 178,205.8 |    2 | 0.6409 |     - |     - |    2688 B |

@inforithmics
Copy link
Contributor

Looking at your benchmark results on the Pull request

SvgColourConverter_Parse_system_colors_single_DIRECT_first seems to be slower in than old implementation
But you have probable already improved the performance since measuring it. So I wonder what are the actual Performance numbers now.

@wieslawsoltes
Copy link
Contributor Author

wieslawsoltes commented Jan 16, 2021

Looking at your benchmark results on the Pull request

SvgColourConverter_Parse_system_colors_single_DIRECT_first seems to be slower in than old implementation
But you have probable already improved the performance since measuring it. So I wonder what are the actual Performance numbers now.

That is special case and really hard to make it faster than original switch (string) as compiler does a really great jog here, but it requires an additional allocation for make string lowercase. My solution does not allocate but it's a bit slower (about 25%). This is only case for SystemColors, that are not that much used (I think) and I have moved those checks as last so they are not so often executed.

I will do benchmarks later, I think net5.0 has some optimizations for that case, I have done a lot to make it fast. The ReadOnlySpan unfortunately does not have switch statement support yet.

@inforithmics
Copy link
Contributor

Looking at your benchmark results on the Pull request
SvgColourConverter_Parse_system_colors_single_DIRECT_first seems to be slower in than old implementation
But you have probable already improved the performance since measuring it. So I wonder what are the actual Performance numbers now.

That is special case and really hard to make it faster than original switch (string) as compiler does a really great jog here, but it requires an additional allocation for make string lowercase. My solution does not allocate but it's a bit slower (about 25%). This is only case for SystemColors, that are not that much used (I think) and I have moved those checks as last so they are not so often executed.

I will do benchmarks later, I think net5.0 has some optimizations for that case, I have done a lot to make it fast. The ReadOnlySpan unfortunately does not have switch statement support yet.

Would it be possible to Make the Lookup Dictionary CaseInsensitive and avoid the ToLowerAscii call?

var caseInsensitiveDictionary = new Dictionary<string,...>(StringComparer.OrdinalIgnoreCase);

@inforithmics
Copy link
Contributor

inforithmics commented Mar 15, 2022

I think this can now be merged because. The Roslyn Compiler can soon do switches on ReadonlySpans
see issue.
dotnet/csharplang#1881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rgb() color values should be clipped
2 participants