Skip to content

Conversation

@antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Apr 20, 2019

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

This PR fixes #642 by limiting the memory consumption of ResizeProcessor to a value defined in Configuration.WorkingBufferSizeHintInBytes. Instead of allocating the whole first pass buffer, a sliding window is implemented by ResizeWorker<T>. The height of the window is a multiple of the vertical kernel's maximum diameter depending on Configuration.WorkingBufferSizeHintInBytes.

The implementation is most likely very different from MagicScaler's one (haven't seen it's code). It was a goal to keep the changes under control, the new code is basically a refactor of the old one.

The optimization is visually explained in ResizeWorker.pptx:

image

image

image

Results

There is no regression in speed. (Actually, we are faster now for .NET 4.72 than before.)

The resizing of a 4000 x 4000 image to 300 x 300 has been memory profiled using JetBrains dotMemory. The resize operation has been executed 50 times in a single process.

Memory profile before the optimization

Timeline:
image
image

Typical snapshot:
image

Memory profile after the optimization

Timeline:
image

image

(The second peak has been only triggered because I made the snapshot. Can anyone explain this?)

Typical snapshot:
image

@codecov
Copy link

codecov bot commented Apr 22, 2019

Codecov Report

Merging #888 into master will increase coverage by 0.04%.
The diff coverage is 89.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #888      +/-   ##
==========================================
+ Coverage   88.98%   89.02%   +0.04%     
==========================================
  Files        1018     1024       +6     
  Lines       44537    44786     +249     
  Branches     3220     3230      +10     
==========================================
+ Hits        39632    39872     +240     
- Misses       4194     4202       +8     
- Partials      711      712       +1
Impacted Files Coverage Δ
...ests/TestUtilities/ImageProviders/SolidProvider.cs 60% <ø> (ø) ⬆️
...nsforms/ResizeKernelMapTests.ReferenceKernelMap.cs 92.3% <0%> (ø) ⬆️
tests/ImageSharp.Tests/Memory/Buffer2DTests.cs 100% <100%> (ø) ⬆️
...PixelFormats/PixelConversionModifiersExtensions.cs 100% <100%> (ø) ⬆️
.../Attributes/WithBasicTestPatternImagesAttribute.cs 100% <100%> (ø)
...ng/Processors/Transforms/Resize/ResizeProcessor.cs 94.78% <100%> (-1.45%) ⬇️
...Tests/TestUtilities/ImageProviders/FileProvider.cs 57.81% <100%> (ø) ⬆️
...estUtilities/ImageProviders/TestPatternProvider.cs 98.01% <100%> (ø) ⬆️
...ng/Processors/Transforms/Resize/ResizeKernelMap.cs 94.73% <100%> (+0.05%) ⬆️
.../TestUtilities/ImageProviders/TestImageProvider.cs 84.78% <100%> (+0.33%) ⬆️
... and 28 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 bcd5f02...58f8d63. Read the comment docs.

@JimBobSquarePants JimBobSquarePants added this to the 1.0.0-rc1 milestone Apr 22, 2019
@antonfirsov antonfirsov merged commit 87c0a66 into master Apr 22, 2019
@JimBobSquarePants JimBobSquarePants deleted the af/resize-sandbox branch April 23, 2019 04:19
antonfirsov added a commit to antonfirsov/ImageSharp that referenced this pull request Nov 11, 2019
Limit ResizeProcessor memory consumption
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.

Optimize memory consumption of ResizeProcessor

4 participants