Load source images asynchronously to prevent thread pool exhaustion

[kroymann]

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

While debugging a memory dump taken during an outage of our production image processing servers, we discovered that nearly all of the thread pool worker threads were blocked waiting on network IO loading source image data, which led to thread pool starvation that was preventing any async work from executing. This PR addresses the source of the problem by replacing the synchronous Image.Load() call with the equivalent async call.

[codecov]

Codecov Report

Merging #222 (0160ec3) into master (9a03ec8) will decrease coverage by 0%.
The diff coverage is 80%.

@@          Coverage Diff          @@
##           master   #222   +/-   ##
=====================================
- Coverage      87%    86%   -1%     
=====================================
  Files          70     70           
  Lines        1824   1828    +4     
  Branches      268    268           
=====================================
- Hits         1589   1588    -1     
- Misses        166    170    +4     
- Partials       69     70    +1     

Flag	Coverage Δ
unittests	86% <80%> (-1%)	⬇️

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

Impacted Files	Coverage Δ
src/ImageSharp.Web/FormattedImage.cs	87% <75%> (-9%)	⬇️
.../ImageSharp.Web/Middleware/ImageSharpMiddleware.cs	85% <100%> (ø)
...ching/LruCache/ConcurrentTLruCache{TKey,TValue}.cs	52% <0%> (-2%)	⬇️

---

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 9a03ec8...0160ec3. Read the comment docs.

[sebastienros]

Should the non-async ones now be made obsolete in the next version (assuming it will be a major one?)

Updated

[kroymann]

Should the non-async ones now be made obsolete in the next version (assuming it will be a major one?)

Probably yes? I'd be willing to be that the vast majority of the time, the image source stream requires network or file I/O to read, and therefore giving people the ability to synchronously load it is almost always going to be the wrong thing to do.

[deanmarcussen]

Looks like you could use SaveAsync as well?

[kroymann]

Looks like you could use SaveAsync as well?

I originally intended to do that, but then realized that the outStream is a RecyclableMemoryStream which means both the source and destination are in-memory buffers and therefore the save operation shouldn't need to do any asynchronous IO.

[JimBobSquarePants]

Perfect, thanks!

We’ll leave the old methods in for now without obsoleting them. I doubt anyone uses them but I think I’d prefer to maintain the sync/async API balance since your using Save()