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

Tiff: Performance improvements for Fax4 decompression #2134

Merged
merged 32 commits into from
Jun 9, 2022
Merged

Conversation

brianpopow
Copy link
Collaborator

@brianpopow brianpopow commented May 29, 2022

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This PR brings performance improvements for Tiff Fax4 decompression.

Related to #2132

Benchmarks:

BenchmarkDotNet=v0.13.0, OS=Windows 10.0.19043.1706 (21H1/May2021Update)
Intel Core i7-6700K CPU 4.00GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
.NET SDK=7.0.100-preview.4.22252.9
  [Host]     : .NET 6.0.5 (6.0.522.21309), X64 RyuJIT
  Job-NMOCEA : .NET 6.0.5 (6.0.522.21309), X64 RyuJIT

Runtime=.NET 6.0  Arguments=/p:DebugType=portable  InvocationCount=1
IterationCount=3  LaunchCount=1  UnrollFactor=1
WarmupCount=3

main

|                Method |                                      TestImage |         Mean |       Error |    StdDev | Ratio | RatioSD |     Gen 0 |     Gen 1 | Gen 2 |    Allocated |
|---------------------- |----------------------------------------------- |-------------:|------------:|----------:|------:|--------:|----------:|----------:|------:|-------------:|
| 'System.Drawing Tiff' |                          Tiff/CCITTGroup4.tiff |   1,529.8 us |   568.26 us |  31.15 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |                          Tiff/CCITTGroup4.tiff | 117,745.8 us | 4,076.57 us | 223.45 us | 76.99 |    1.64 |         - |         - |     - |     20,872 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |    Tiff/Calliphora_grayscale_uncompressed.tiff |     196.0 us |   774.67 us |  42.46 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |    Tiff/Calliphora_grayscale_uncompressed.tiff |     176.2 us |   175.31 us |   9.61 us |  0.94 |    0.26 |         - |         - |     - |     22,688 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |     Tiff/Calliphora_rgb_deflate_predictor.tiff |   1,195.0 us |   551.90 us |  30.25 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |     Tiff/Calliphora_rgb_deflate_predictor.tiff |   1,082.5 us |   424.99 us |  23.30 us |  0.91 |    0.03 |         - |         - |     - |     23,856 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |         Tiff/Calliphora_rgb_lzw_predictor.tiff |   1,212.8 us |   663.35 us |  36.36 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |         Tiff/Calliphora_rgb_lzw_predictor.tiff |   5,099.8 us | 3,391.88 us | 185.92 us |  4.20 |    0.03 |         - |         - |     - |  2,935,984 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |              Tiff/Calliphora_rgb_packbits.tiff |     472.1 us |   209.06 us |  11.46 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |              Tiff/Calliphora_rgb_packbits.tiff |     206.3 us |   364.32 us |  19.97 us |  0.44 |    0.04 |         - |         - |     - |     22,840 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' | Tiff/Calliphora_rgb_palette_lzw_predictor.tiff |  34,671.3 us | 8,561.18 us | 469.27 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' | Tiff/Calliphora_rgb_palette_lzw_predictor.tiff |  43,881.6 us | 1,559.07 us |  85.46 us |  1.27 |    0.02 | 4000.0000 | 1000.0000 |     - | 18,032,064 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |          Tiff/Calliphora_rgb_uncompressed.tiff |     269.9 us |   114.12 us |   6.26 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |          Tiff/Calliphora_rgb_uncompressed.tiff |     170.5 us |   247.64 us |  13.57 us |  0.63 |    0.04 |         - |         - |     - |     22,736 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |     Tiff/ccitt_fax3_all_terminating_codes.tiff |     138.0 us |   881.60 us |  48.32 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |     Tiff/ccitt_fax3_all_terminating_codes.tiff |     493.3 us |   407.67 us |  22.35 us |  3.80 |    0.98 |         - |         - |     - |     19,320 B |
|                       |                                                |              |             |           |       |         |           |           |       |              |
| 'System.Drawing Tiff' |         Tiff/huffman_rle_all_makeup_codes.tiff |     118.6 us |   219.62 us |  12.04 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |         Tiff/huffman_rle_all_makeup_codes.tiff |   2,893.1 us |    46.58 us |   2.55 us | 24.56 |    2.41 |         - |         - |     - |     19,472 B |

PR

|                Method |                                      TestImage |        Mean |        Error |    StdDev |      Median | Ratio | RatioSD |     Gen 0 |     Gen 1 | Gen 2 |    Allocated |
|---------------------- |----------------------------------------------- |------------:|-------------:|----------:|------------:|------:|--------:|----------:|----------:|------:|-------------:|
| 'System.Drawing Tiff' |                          Tiff/CCITTGroup4.tiff |  1,487.8 us |    444.38 us |  24.36 us |  1,480.4 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |                          Tiff/CCITTGroup4.tiff | 12,071.3 us |  5,696.77 us | 312.26 us | 11,972.3 us |  8.12 |    0.25 |         - |         - |     - |     20,776 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |    Tiff/Calliphora_grayscale_uncompressed.tiff |    166.0 us |    199.36 us |  10.93 us |    162.9 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |    Tiff/Calliphora_grayscale_uncompressed.tiff |    187.2 us |    424.07 us |  23.24 us |    174.3 us |  1.13 |    0.07 |         - |         - |     - |     22,688 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |     Tiff/Calliphora_rgb_deflate_predictor.tiff |  1,080.1 us |    150.10 us |   8.23 us |  1,084.8 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |     Tiff/Calliphora_rgb_deflate_predictor.tiff |  1,085.6 us |  1,065.06 us |  58.38 us |  1,055.3 us |  1.01 |    0.05 |         - |         - |     - |     23,856 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |         Tiff/Calliphora_rgb_lzw_predictor.tiff |  1,365.7 us |  1,413.47 us |  77.48 us |  1,350.8 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |         Tiff/Calliphora_rgb_lzw_predictor.tiff |  5,756.3 us | 14,228.63 us | 779.92 us |  5,611.7 us |  4.21 |    0.42 |         - |         - |     - |  2,935,984 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |              Tiff/Calliphora_rgb_packbits.tiff |    491.7 us |    258.43 us |  14.17 us |    490.9 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |              Tiff/Calliphora_rgb_packbits.tiff |    199.0 us |    349.29 us |  19.15 us |    190.4 us |  0.41 |    0.04 |         - |         - |     - |     22,840 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' | Tiff/Calliphora_rgb_palette_lzw_predictor.tiff | 34,776.5 us |  6,158.07 us | 337.54 us | 34,647.3 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' | Tiff/Calliphora_rgb_palette_lzw_predictor.tiff | 43,805.4 us |  2,389.42 us | 130.97 us | 43,846.3 us |  1.26 |    0.01 | 4000.0000 | 1000.0000 |     - | 18,032,064 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |          Tiff/Calliphora_rgb_uncompressed.tiff |    300.3 us |    103.24 us |   5.66 us |    300.7 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |          Tiff/Calliphora_rgb_uncompressed.tiff |    157.9 us |    159.64 us |   8.75 us |    153.1 us |  0.53 |    0.03 |         - |         - |     - |     22,736 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |     Tiff/ccitt_fax3_all_terminating_codes.tiff |    169.5 us |  1,506.78 us |  82.59 us |    122.4 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |     Tiff/ccitt_fax3_all_terminating_codes.tiff |    218.3 us |    416.90 us |  22.85 us |    223.8 us |  1.51 |    0.67 |         - |         - |     - |     19,224 B |
|                       |                                                |             |              |           |             |       |         |           |           |       |              |
| 'System.Drawing Tiff' |         Tiff/huffman_rle_all_makeup_codes.tiff |    114.8 us |     83.39 us |   4.57 us |    117.1 us |  1.00 |    0.00 |         - |         - |     - |        648 B |
|     'ImageSharp Tiff' |         Tiff/huffman_rle_all_makeup_codes.tiff |    617.8 us |    115.51 us |   6.33 us |    614.5 us |  5.39 |    0.28 |         - |         - |     - |     19,088 B |

note: Tiff/CCITTGroup4.tiff is the image from #2132

Sorry, something went wrong.

@brianpopow brianpopow changed the title Tiff: Performance improvements for Fax4 decompression WIP: Tiff: Performance improvements for Fax4 decompression May 29, 2022
@TonyValenti
Copy link

@brianpopow -
I was looking at this code and I noticed something I wanted to mention to you.

I noticed that there seem to be a number of "constant" dictionaries that are used.

I've seen many situations where dictionaries like that are refactored into a static switch method. This provides better performance because the C# compiler will optimize each switch statement instead of relying on the General purpose Dictionary Class.

Wanted to mention that because it may further reduce memory as well.

@TonyValenti
Copy link

Err, further reduce execution time.

brianpopow and others added 12 commits May 29, 2022 17:15

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@brianpopow
Copy link
Collaborator Author

brianpopow commented May 30, 2022

@brianpopow - I was looking at this code and I noticed something I wanted to mention to you.

I noticed that there seem to be a number of "constant" dictionaries that are used.

I've seen many situations where dictionaries like that are refactored into a static switch method. This provides better performance because the C# compiler will optimize each switch statement instead of relying on the General purpose Dictionary Class.

Wanted to mention that because it may further reduce memory as well.

So far we are down from 117,745.8 us to 15,028.70 us.
I think that this is not bad, but the difference to System.Drawing is with 1,582.76 us still quite a bit. I am currently out of idea's how to improve that further.
I tried avoiding dictionary's with d5a3de1, it's only a very small difference unfortunately.

edit: with 10ff24f we are down to 12,071.3 us

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@brianpopow brianpopow changed the title WIP: Tiff: Performance improvements for Fax4 decompression Tiff: Performance improvements for Fax4 decompression May 31, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Co-authored-by: Berkan Diler <b.diler@gmx.de>
@br3aker
Copy link
Contributor

br3aker commented Jun 1, 2022

I'll take a closer look and play with benchmarks when this is shipped but I don't understand atm why bits are pushed bit by bit when we can have a lookup table or maybe a somehow optimized byte value generation.

@brianpopow
Copy link
Collaborator Author

brianpopow commented Jun 2, 2022

I'll take a closer look and play with benchmarks when this is shipped but I don't understand atm why bits are pushed bit by bit when we can have a lookup table or maybe a somehow optimized byte value generation.

Thanks @br3aker, it would be good to have another pair of eyes looking at this, since I am not sure how or if this can be improved further. I will mark it now as Ready for review.

Nevertheless I think the improvement compared to main is already quite significant.

edit: btw: huffman and fax3 decompression also benefit from this PR.

@brianpopow brianpopow requested a review from a team June 2, 2022 08:59
@br3aker
Copy link
Contributor

br3aker commented Jun 2, 2022

Nevertheless I think the improvement compared to main is already quite significant.

90% speedup is simply incredible :)

I believe we can win a lot in those bit insertions though.

brianpopow and others added 4 commits June 3, 2022 15:31
- Deflate and packbits are not supported by System.Drawing
- When 1d Compression is chosen, TiffPhotometricInterpretation should be WhiteIsZero
@brianpopow brianpopow merged commit a194b9e into main Jun 9, 2022
@brianpopow brianpopow deleted the bp/Issue2132 branch June 9, 2022 14:21
@brianpopow brianpopow mentioned this pull request Jun 9, 2022
4 tasks
@TonyValenti
Copy link

@brianpopow Thank you so much for your work on this.

Do you have the ability to publish an updated nuget package? If so, can you make that happen?

@brianpopow
Copy link
Collaborator Author

@TonyValenti only new stable release versions and hotfix versions will be published via Nuget, but you can use our MyGet feed for alpha releases: https://www.myget.org/feed/sixlabors/package/nuget/SixLabors.ImageSharp

Just add this as an additional package source and you are good to go.

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

Successfully merging this pull request may close these issues.

None yet

5 participants