#12 Synch with master and improve performance of decoders #986

IldarKhayrutdinov · 2019-08-22T13:14:04Z

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 matches 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

merge from master
add simple blackBox tests and benchmarks for decoder
report deflate decoding bug (see: 51720a8)
improve performance of decoders at 1.1x - 2.5x (see: 1b32629)

* Update metadata names * Use WithIterationCount * Format Benchmark documents * Update copyright assignment to Six Labors & Contributors * Update deps * React to Benchmark library update

- ResizeWindow -> ResizeWorker - Most logic moved to ResizeWorker

which has been renamed from CalculateResizeWorkerWindowCount()

…nit-tests for Tiff decoder.

…s#979) * remove some redundant constructor overloads from exceptions * re add ImageProcessingException ctor only used in release

… tiff-codec # Conflicts: # src/ImageSharp/Configuration.cs # src/ImageSharp/PixelFormats/PixelBlenders/DefaultPixelBlenders.Generated.cs # src/ImageSharp/PixelFormats/PixelBlenders/DefaultPixelBlenders.Generated.tt # src/ImageSharp/PixelFormats/PixelBlender{TPixel}.cs # tests/ImageSharp.Tests/ConfigurationTests.cs

Correct Metadata updated name (for Resolution properties), temporary disable tiff native metadata properties.

antonfirsov · 2019-08-22T23:22:27Z

@Sergio0694 can you help a bit? Doest the bokeh blur processor + test code look up to date on this branch?

EDIT:
Or does it need a submodule update to get the latest reference images?

James-Jackson-South · 2019-08-23T03:55:14Z

@antonfirsov @Sergio0694 I can have a look, I'm familiar with that code also now.

IldarKhayrutdinov · 2019-08-23T07:26:02Z

@antonfirsov @Sergio0694 Thanks! Really I forgot to update submodules 😃
@antonfirsov @James-Jackson-South @Andy-Wilkinson Now I have passed the tests locally and I expect that on the CI will pass. Please code review.
I continue to work on Tiff format, soon I will prepare new PR.

Sergio0694 · 2019-08-23T09:59:47Z

Thanks @James-Jackson-South!
@antonfirsov I think James is actually more familiar than me with the tests part of the bokeh blur processor actually, as he kindly set that part up after I finished working on the last tweaks for the processor itself 😄

JimBobSquarePants · 2019-08-23T10:06:09Z

1198 files! Crikey!

I'll do a folder check via BeyondCompare before digging through the diff. Everything should be identical except for the new Tiff code.

codecov · 2019-08-23T10:09:18Z

Codecov Report

Merging #986 into tiff-codec will increase coverage by 0.87%.
The diff coverage is 91.1%.

@@              Coverage Diff               @@
##           tiff-codec     #986      +/-   ##
==============================================
+ Coverage       89.08%   89.96%   +0.87%     
==============================================
  Files            1084     1168      +84     
  Lines           47212    51664    +4452     
  Branches         3402     3644     +242     
==============================================
+ Hits            42061    46481    +4420     
- Misses           4391     4447      +56     
+ Partials          760      736      -24

Impacted Files	Coverage Δ
...ImageSharp/Formats/Tiff/TiffImageFormatDetector.cs	`100% <ø> (ø)`	⬆️
...PhotometricInterpretation/BlackIsZero4TiffColor.cs	`100% <ø> (ø)`	⬆️
src/ImageSharp/Formats/Jpeg/JpegMetaData.cs	`100% <ø> (ø)`	⬆️
src/ImageSharp/ImageFrame.cs	`100% <ø> (ø)`
src/ImageSharp/Formats/Jpeg/JpegConstants.cs	`100% <ø> (ø)`	⬆️
.../Implementation/WorkingSpaces/GammaWorkingSpace.cs	`29.41% <ø> (ø)`	⬆️
...ImageSharp/Formats/Tiff/TiffConfigurationModule.cs	`100% <ø> (+100%)`	⬆️
src/ImageSharp/Formats/Jpeg/JpegFormat.cs	`87.5% <ø> (ø)`	⬆️
src/ImageSharp/Memory/Buffer2D{T}.cs	`100% <ø> (ø)`	⬆️
src/ImageSharp/ImageExtensions.Internal.cs	`100% <ø> (ø)`	⬆️
... and 882 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 904f5be...b576c25. Read the comment docs.

codecov · 2019-08-23T10:09:19Z

Codecov Report

Merging #986 into tiff-codec will increase coverage by 0.96%.
The diff coverage is 91.1%.

@@              Coverage Diff               @@
##           tiff-codec     #986      +/-   ##
==============================================
+ Coverage       89.08%   90.05%   +0.96%     
==============================================
  Files            1084     1169      +85     
  Lines           47212    51679    +4467     
  Branches         3402     3644     +242     
==============================================
+ Hits            42061    46540    +4479     
- Misses           4391     4405      +14     
+ Partials          760      734      -26

Impacted Files	Coverage Δ
...ImageSharp/Formats/Tiff/TiffImageFormatDetector.cs	`100% <ø> (ø)`	⬆️
...PhotometricInterpretation/BlackIsZero4TiffColor.cs	`100% <ø> (ø)`	⬆️
src/ImageSharp/Formats/Jpeg/JpegMetaData.cs	`100% <ø> (ø)`	⬆️
src/ImageSharp/ImageFrame.cs	`100% <ø> (ø)`
src/ImageSharp/Formats/Jpeg/JpegConstants.cs	`100% <ø> (ø)`	⬆️
.../Implementation/WorkingSpaces/GammaWorkingSpace.cs	`29.41% <ø> (ø)`	⬆️
...ImageSharp/Formats/Tiff/TiffConfigurationModule.cs	`100% <ø> (+100%)`	⬆️
src/ImageSharp/Formats/Jpeg/JpegFormat.cs	`87.5% <ø> (ø)`	⬆️
src/ImageSharp/Memory/Buffer2D{T}.cs	`100% <ø> (ø)`	⬆️
src/ImageSharp/ImageExtensions.Internal.cs	`100% <ø> (ø)`	⬆️
... and 885 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 904f5be...5a601ef. Read the comment docs.

IldarKhayrutdinov · 2019-08-24T14:32:13Z

1198 files! Crikey!

I'll do a folder check via BeyondCompare before digging through the diff. Everything should be identical except for the new Tiff code.

Sure, the most of changes are merge from master.

antonfirsov · 2019-08-24T21:09:06Z

@James-Jackson-South since this is not going into main, I think we can merge it as-is as soon as the tests in TiffDecoderTests are matching the current capabilities of the decoder.

I see a few improvement points, but I don't think it's a good idea to address them in this huge PR, let's do them later:

Using MemoryAllocator and disposable buffers instead of ArrayPool.Shared
Consider sharing code with other decoders. Eg. TiffLzwDecoder looks pretty much the same as SixLabors.ImageSharp.Formats.Gif.LzwDecoder

antonfirsov · 2019-08-24T21:20:50Z

@James-Jackson-South see #988 for the diff.

JimBobSquarePants · 2019-08-25T02:32:00Z

Thanks, I’ll have a look later today. 👍

JimBobSquarePants · 2019-08-25T13:34:59Z

Looks good to me. Will merge now.

iamcarbon and others added 30 commits April 14, 2019 12:01

Nits - Benchmarks (SixLabors#884)

e9b9e27

* Update metadata names * Use WithIterationCount * Format Benchmark documents * Update copyright assignment to Six Labors & Contributors * Update deps * React to Benchmark library update

ResizeWindowOld

4d38d7c

ResizeWindow refactor 1

f81939e

ResizeWindow refactor 2

7079410

ResizeWindow refactor 3

545abf2

ResizeWindow refactor 4

20ddd11

basic sliding window implementation (semi-stable state)

86cc83d

fix ResizeWithCropHeightMode

56de415

reference output for Resize_BasicSmall

8e607ae

minor optimization

682e521

Handle incorrect colorspace metadata. Fix SixLabors#882 (SixLabors#885)

5a85ea7

refactor

4a0839a

- ResizeWindow -> ResizeWorker - Most logic moved to ResizeWorker

refactor stuff + implement CalculateResizeWorkerWindowCount()

b3495e8

utilize CalculateResizeWorkerHeightInWindowBands()

40aea16

which has been renamed from CalculateResizeWorkerWindowCount()

improve benchmark: ArrayCopy -> CopyBuffers

15b3b2a

moar RowInterval stuff

6db0701

buffer.CopyColumns(...)

19766df

simplify ResizeWorker logic

d9e4d6c

WorkingBufferSizeHintInBytes_IsAppliedCorrectly

51cc659

more robust tests

3860fb6

ResizeTests.LargeImage

1fd135f

optimized sliding works!

f6c7784

reapply unsafe optimizations

f7e0a94

moar unsafe optimization

2168071

benchmark WorkingBufferSizeHint effects

a7c5295

memory profiling with Sandbox46

d882d91

add ResizeWorker.pptx

0c684ab

refine ResizeWorker.pptx

1ab291c

xmldoc for ResizeWorker

94b52bf

update ResizeWorker.pptx [skip CI]

d2e1a8d

IldarKhayrutdinov and others added 9 commits August 22, 2019 10:17

Add TiffConfigurationModule to core Configuration. Implement simple u…

5e4c90f

…nit-tests for Tiff decoder.

Add benchmarks for big and medium tiff files

104cac1

Move Tiff classes to Formats.Tiff namespace

ab55bef

Mark Tiff format tests with "Tiff" category, cleanup test classes

e101736

Improve performance of Tiff decoders

1b32629

remove some redundant constructor overloads from exceptions (SixLabor…

79f5ff5

…s#979) * remove some redundant constructor overloads from exceptions * re add ImageProcessingException ctor only used in release

Fix of build error, cleanup.

f75a17b

Correct Metadata updated name (for Resolution properties), temporary disable tiff native metadata properties.

Implement temporary tiff native metadata structures

e8d45db

IldarKhayrutdinov changed the title ~~WIP: Synch with master and improve performance of decoders~~ WIP: #12 Synch with master and improve performance of decoders Aug 22, 2019

Mark missed tiff tests, exclude DecodeManual test

ae820db

Update test images submodule

5a601ef

antonfirsov mentioned this pull request Aug 24, 2019

Difference between our current master and IldarKhayrutdinov:tiff-codec #988

Closed

Explanations

b576c25

JimBobSquarePants merged commit 9773e61 into SixLabors:tiff-codec Aug 25, 2019

IldarKhayrutdinov changed the title ~~WIP: #12 Synch with master and improve performance of decoders~~ #12 Synch with master and improve performance of decoders Aug 26, 2019

IldarKhayrutdinov deleted the tiff-codec branch August 26, 2019 12:30

JimBobSquarePants mentioned this pull request Aug 20, 2020

WIP #12 Refactory tiff structures, decoder classes #1322

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#12 Synch with master and improve performance of decoders #986

#12 Synch with master and improve performance of decoders #986

IldarKhayrutdinov commented Aug 22, 2019 •

edited

Loading

antonfirsov commented Aug 22, 2019 •

edited

Loading

James-Jackson-South commented Aug 23, 2019

IldarKhayrutdinov commented Aug 23, 2019 •

edited

Loading

Sergio0694 commented Aug 23, 2019

JimBobSquarePants commented Aug 23, 2019

codecov bot commented Aug 23, 2019 •

edited

Loading

codecov bot commented Aug 23, 2019

IldarKhayrutdinov commented Aug 24, 2019

antonfirsov commented Aug 24, 2019

antonfirsov commented Aug 24, 2019

JimBobSquarePants commented Aug 25, 2019

JimBobSquarePants commented Aug 25, 2019

#12 Synch with master and improve performance of decoders #986

#12 Synch with master and improve performance of decoders #986

Conversation

IldarKhayrutdinov commented Aug 22, 2019 • edited Loading

Prerequisites

Description

antonfirsov commented Aug 22, 2019 • edited Loading

James-Jackson-South commented Aug 23, 2019

IldarKhayrutdinov commented Aug 23, 2019 • edited Loading

Sergio0694 commented Aug 23, 2019

JimBobSquarePants commented Aug 23, 2019

codecov bot commented Aug 23, 2019 • edited Loading

Codecov Report

codecov bot commented Aug 23, 2019

Codecov Report

IldarKhayrutdinov commented Aug 24, 2019

antonfirsov commented Aug 24, 2019

antonfirsov commented Aug 24, 2019

JimBobSquarePants commented Aug 25, 2019

JimBobSquarePants commented Aug 25, 2019

IldarKhayrutdinov commented Aug 22, 2019 •

edited

Loading

antonfirsov commented Aug 22, 2019 •

edited

Loading

IldarKhayrutdinov commented Aug 23, 2019 •

edited

Loading

codecov bot commented Aug 23, 2019 •

edited

Loading