Is ImageSharp thread safe? - Document concurrent behavior about docs HOT 12 CLOSED

@JimBobSquarePants I transferred this to the docs repo as a reminder.

I think we need an entry with title "Is ImageSharp thread safe" or similar, and elaborate major points from #11 (comment).

from docs.

I checked this morning and the example program reproduces the problems on windows as well

PS D:\Test\ParallelImage> dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.100
 Commit:    cd82f021f4

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.100\

Host (useful for support):
  Version: 3.1.0
  Commit:  65f04fb6db

.NET Core SDKs installed:
  1.1.14 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.2.107 [C:\Program Files\dotnet\sdk]
  2.2.301 [C:\Program Files\dotnet\sdk]
  2.2.401 [C:\Program Files\dotnet\sdk]
  2.2.402 [C:\Program Files\dotnet\sdk]
  3.0.100-preview9-014004 [C:\Program Files\dotnet\sdk]
  3.1.100 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview9.19424.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

from docs.

The we don't currently deem the APIs on Image, Image<T> or the IImageProcessingContext as thread safe as the underlying apis are inherently destructive.

Also All the methods called inside the .Mutate(..)/.Clone(..) actions are not processed until the method exits are are enqueueing a processing pipeline to execute not running a series of methods one after each other.

from docs.

Thanks for the info, I had assumed that these methods would have been thread safe since I found this in the readme of ImageSharp

Gone are system-wide process-locks; ImageSharp images are thread-safe and fully supported in web environments.

I'm just now noticing that that is a (related but) different package, you can see I created this issue in the wrong repo.

Do you know where I can obtain a list of which parts of the ImageSharp and ImageSharp.Drawing libraries are thread safe?

from docs.

ImageSharp images are thread-safe

That means the Image and Image<TPixels> classes are thread safe, not that individual processing algorithms are thread safe.

You can get/set an image pixels in parallel without locking and you can process multiple image in parallel. (System.Drawing creates a global lock on GetPixel() and SetPixel()).

Think about it like this, certain image processing operations require an interim copy of either the total or a portion of the source image in order to operate. Once you start applying those operations in parallel you create multiple copies and or could be accessing areas of an image already updated. It gets messy very quickly.

Some like Error diffusion, for example, are generally considered a serial algorithm since each pixel value depends on changes to surrounding pixels. (Note there is a parallel strategy that can be applied to error diffusion but that requires structured window sampling and cannot be implemented in the manner described above).

Certain operations would work if there is no copy required but there are not that many of them.

Many of our processing operation run internally in parallel on the image using carefully crafted rules to determine the correct options to supply to Parallel in order to run at maximum speed and not exhaust threads. Running many operations on an image in parallel (if the algorithm allows it) may not actually improve performance.

from docs.

Thank you again for the quick and informative response. Since this is working as intended should we close the issue? Would it make sense to update the docs to make it clear which operations are thread safe? I understand that may not not necessarily be a simply tasks, and it may not be a high priority when compared to other initiatives of your project.

from docs.

That means the Image and Image classes are thread safe, not that individual processing algorithms are thread safe.

@JimBobSquarePants I think this is not correct, since not all API-s of Image (and related classes) are safe for concurrent modifications.

This is my interpretation:

• The lack of global locks ≠ thread safety (but still a big advantage compared to System.Drawing/GDI+)
• In general, no classes / API-s should be considered fully thread safe in ImageSharp, since the vast majority only allows safe concurrent reads. Concurrent R+W or W+W is not supported.
• However: you can still do certain "common sense" primitive operations concurrently, if you are careful. Eg. writing to image.GetRowSpan(y1) from one thread and to image.GetRowSpan(y2) from another one.
• Certain classes are do caching/pooling internally during their read operations. (eg. MemoryAllocator or the internal BokehBlurKernelDataProvider ). All those state-modifying operations should be safe for concurrent access, and we should probably explicitly test for this.

from docs.

@antonfirsov Note that the current implementation of the bokeh blur cache is not perfectly thread safe. I mean, it is in the sense that you can do multiple operations in parallel just fine without issues, but there's a slight chance that perfectly concurrent requests on an initial non-cached kernel might result in the same kernel being recomputed more than once (ie. once in each concurrent thread). Operations will still proceed just fine after that, as the computed values from the threads after the first one will just gracefully be discarded.

Despite this downside, this implementation has the advantage of not requiring to hold a lock on the entire cache mapping while any given kernel is being computed.

I guess there are pros and cons to both techniques, if you'd like me to revisit this feel free to ping me and I'll be happy to discuss this further with you guys on gitter 👍

from docs.

@Sergio0694 this is an implementation detail, since it does not impact observable behavior. I think you solution is fine, you made a good choice.

from docs.

@antonfirsov Good point.

Concurrent R+W or W+W is not supported.

We should update the copy in the readme. If anyone has any suggestions as what to replace it with to make things less confusing, I'm all ears.

from docs.

@JimBobSquarePants we should probably add a point under docs.sixlabors.com/articles

Side note:
We should consider migrating articles to ImageSharp main repo and link them from README.md for better visibility.

from docs.

Updated to remove statement.

from docs.