Skip to content

Conversation

@antonfirsov
Copy link
Member

@antonfirsov antonfirsov commented Jun 10, 2020

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

Integrating FixedCapacityPooledMemoryStream with MemoryAllocator and moving duplicate async decoder code to a utility class.

ArrayPool.Shared is good when (1) the rented buffer is relatively small (2) the Rent operation is a bottleneck on a hot path. In image processing these conditions are barely true. For a byte array > 1MB, the shared pool is always allocating a new array, which is a common for raw image streams.

@codecov
Copy link

codecov bot commented Jun 10, 2020

Codecov Report

Merging #1223 into master will increase coverage by 0.14%.
The diff coverage is 79.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1223      +/-   ##
==========================================
+ Coverage   82.40%   82.54%   +0.14%     
==========================================
  Files         696      697       +1     
  Lines       30553    30506      -47     
  Branches     3482     3467      -15     
==========================================
+ Hits        25176    25181       +5     
+ Misses       4658     4606      -52     
  Partials      719      719              
Flag Coverage Δ
#unittests 82.54% <79.03%> (+0.14%) ⬆️
Impacted Files Coverage Δ
src/ImageSharp/Formats/ImageDecoderUtilities.cs 28.57% <28.57%> (ø)
src/ImageSharp/Image.FromStream.cs 76.56% <50.00%> (ø)
src/ImageSharp/Formats/Gif/GifDecoderCore.cs 83.96% <85.71%> (+4.76%) ⬆️
src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs 92.89% <88.88%> (+3.21%) ⬆️
src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs 94.45% <100.00%> (+0.91%) ⬆️
src/ImageSharp/Formats/Png/PngDecoderCore.cs 90.63% <100.00%> (+2.53%) ⬆️
src/ImageSharp/Formats/Tga/TgaDecoderCore.cs 91.29% <100.00%> (+3.71%) ⬆️
...c/ImageSharp/IO/FixedCapacityPooledMemoryStream.cs 100.00% <100.00%> (ø)
src/ImageSharp/Memory/MemoryAllocatorExtensions.cs 100.00% <100.00%> (ø)

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 b29bb74...481241d. Read the comment docs.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

@JimBobSquarePants JimBobSquarePants merged commit bbf3233 into master Jun 10, 2020
@JimBobSquarePants JimBobSquarePants deleted the af/FixedCapacityPooledMemoryStream-use-allocator branch June 10, 2020 17:30
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.

3 participants