Fix Zero Length Images

[deanmarcussen]

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

Fixes #158

As discussed on the issue the ConcurrentDictionary arrangement introduced to replace the AsyncKeyLock was misbehaving, and not always writing a response, when under high load.

This pr rearranges the workers to always return an ImageWorkResult from which a response will be written.

It also replaces Lazy with what is recommended as a better way of controlling async ConcurrentDictionary GetOrAdds

Performance numbers drop unfortunately, but still remain better than the AsyncKeyLock
This pr : Reqs/sec 6943.78
Master : Reqs/sec 41455.71
AsyncKeyLock : Reqs/sec 2299.19 (master at aa1b192)

Note: Not rushing this, there's a couple of areas worth looking at a bit further, I will add some comments to the pr

[codecov]

Codecov Report

Merging #160 (76a781f) into master (8f67aae) will decrease coverage by 0.49%.
The diff coverage is 88.03%.

@@            Coverage Diff             @@
##           master     #160      +/-   ##
==========================================
- Coverage   84.82%   84.32%   -0.50%     
==========================================
  Files          46       48       +2     
  Lines        1364     1410      +46     
  Branches      177      187      +10     
==========================================
+ Hits         1157     1189      +32     
- Misses        159      166       +7     
- Partials       48       55       +7     

Flag	Coverage Δ
unittests	84.32% <88.03%> (-0.50%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...p.Web/Middleware/ConcurrentDictionaryExtensions.cs	50.00% <50.00%> (ø)
.../ImageSharp.Web/Middleware/ImageSharpMiddleware.cs	85.92% <91.66%> (-1.96%)	⬇️
src/ImageSharp.Web/Middleware/ImageWorkerResult.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 8f67aae...76a781f. Read the comment docs.

[JimBobSquarePants]

Performance numbers drop unfortunately, but still remain better than the AsyncKeyLock
This pr : Reqs/sec 6943.78
Master : Reqs/sec 41455.71
AsyncKeyLock : Reqs/sec 2299.19 (master at aa1b192)

Ooft that stings! Better a working slower solution but ow!

[deanmarcussen]

Ooft that stings! Better a working slower solution but ow!

I am gutted :(

[JimBobSquarePants]

I'm gonna have a deeper look at the issue here. I'm wondering if there's a less drastic fix.

[deanmarcussen]

That would be great @JimBobSquarePants this needs as many eyes as possible.

I've had a final run through with this pr, and am happy with it.

[mariojsnunes]

Hi, when can we expect a release with this? The memory leak fix is really important :)

[JimBobSquarePants]

I’m awaiting confirmation in the discussion here from a support license holder.

#161

[deanmarcussen]

@mariojsnunes the release is available on the myget feed https://www.myget.org/feed/sixlabors/package/nuget/SixLabors.ImageSharp.Web/1.0.3-alpha.0.4

It would be great to get any feedback on it.

[mariojsnunes]

Tested with OrchardCore on an Azure App Service.
It seems to be solved.
Memory graph increases but goes down right after (5min). Before it would stay up until a server restart.

• First test requesting with 100 big images at the same time (~10mb) the memory increases a lot but returns to normal.
• Second test with 250 small images (~250kb) memory changes are barely noticeable.
• Requesting images after them being server-side cached has no impact on memory.

[JimBobSquarePants]

Thanks for the feedback @mariojsnunes very useful!

I'll get a NuGet release issued ASAP. there's little point holding out.

[markmccaigue]

Hi folks, I've also just completed some testing of the latest packages from MyGet, and can confirm that they appear to have fixed the issue I raised in #158, magic!

[JimBobSquarePants]

Ace, thanks for reporting back!

[JimBobSquarePants]

New NuGet release available. https://www.nuget.org/packages/SixLabors.ImageSharp.Web/1.0.3