SVG icon indicating copy to clipboard operation
SVG copied to clipboard

Performance optimizations for SvgColourConverter parser

Open wieslawsoltes opened this issue 3 years ago • 4 comments

Reference Issue

Split from #786 Fixes: https://github.com/svg-net/SVG/issues/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 |

wieslawsoltes avatar Jan 14 '21 18:01 wieslawsoltes

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.

inforithmics avatar Jan 16 '21 10:01 inforithmics

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.

wieslawsoltes avatar Jan 16 '21 13:01 wieslawsoltes

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 avatar Jan 19 '21 14:01 inforithmics

I think this can now be merged because. The Roslyn Compiler can soon do switches on ReadonlySpans see issue. https://github.com/dotnet/csharplang/issues/1881

inforithmics avatar Mar 15 '22 12:03 inforithmics