2

I have a problem with a threadpool efficiency. I'm not sure I understand the whole concept. I did a lot of reading before asking that question and I know that threadpool is a good solution if you have a lot of small, relatively quick functions AND what's more important - non-blocking tasks. Using lock is very bad in threadpool.

And here is my question: How to return values from threadpool functions? If you have functions to run they probably produce some results, right? It's good to store those results somewhere. Where?

I'm running c.a. 200k very quick functions in a threadpool. The results I store in the List. Of course I have to do:

lock(lockobj)
{
    myList.Add(result);
}

So, is this the right way? I mean, if your functions returns SOMETHING, you have to store them in some kind of collection. It has to be a blocking collection. So, I started thinking... "Blocking is very bead in threadpool, but you have to do this, at least once - at the end of every function

How to store/return results from functions running in threadpool?

Thanks! JB

EDIT: By "function" I mean...

 ThreadPool.QueueUserWorkItem(state =>
 {
    Result r = function(); // previously named "Task"
    lock(lockobj)
    {
        allResults.Add(r);
    }
 }
Bem
  • 21
  • 3
  • yes its the right way. Time it, this should be your baseline for any perf tuning. If you find lock free ways measure them too. Always measure if you have perf tuning ideas. Always have a baseline (simplest working code) – pm100 Nov 22 '16 at 22:05
  • Have you tried `ConcurrentBag` ? – atp9 Nov 22 '16 at 22:06
  • You can use `ConcurrentQueue` if you have any doubts if you can't write correctly the locks. – mybirthname Nov 22 '16 at 22:06
  • You should almost never need to actually access the thread pool directly; you should be using higher level tools that are built on top of it. Which tool you use is going to depend on what your actual higher order operation is. – Servy Nov 22 '16 at 22:11
  • @Bem, you are using `Task` and `ThreadPool` interchangeably, do you understand they are different things? A `Task` may end up as a thread, but most likely it will not. This is managed by the .NET framework. As for the thread-safe collections, the previous commenters have already answered. – hyankov Nov 22 '16 at 22:20
  • @Servy I'm sorry, but your comment is not very clear to me. What do you mean by "access the thread pool directly"? Can you name one tool you meant? Also.. "higher level tools that are built on top of it", top of what? threadpool? – Bem Nov 22 '16 at 22:22
  • He means: start a `Task`, not a `Thread`. – hyankov Nov 22 '16 at 22:24
  • @HristoYankov No, I didn't mean "Task" as a .NET Task (the class) - I mean the english word "Task" as "a thing to do.. a function I should say". Sorry – Bem Nov 22 '16 at 22:24
  • Then that's wrong, you shouldn't do it, as per my comments above. – hyankov Nov 22 '16 at 22:24
  • @HristoYankov Well, the `ThreadPool`, not a `Thread`, but yes. – Servy Nov 22 '16 at 22:25
  • @HristoYankov I'm not sure I understand what you mean.. What `Task`s? what `Thread`s? Please have a look at my edited question.. – Bem Nov 22 '16 at 22:52
  • @atp9 `ConcurrentBag` is not a good solution here for performance reasons because it uses thread local storage. You're better off with a different collection from `System.Collections.Concurrent` such as `ConcurrentQueue`. Your suggestion will work, but at a costly performance hit. The MSDN documentation specifies when this collection is best used and this is definitely not it. – Zer0 Nov 22 '16 at 23:16

2 Answers2

1

If you don't want to block the ThreadPool threads use a lock-free approach. ConcurrentQueue is currently lock-free (as of .NET 4.6.2) when you enqueue items.

So simply do this:

public static ConcurrentQueue<Result> AllResults { get; } = new ConcurrentQueue<Result>();

ThreadPool.QueueUserWorkItem(state =>
{
    Result r = function();
    AllResults.Enqueue(r);
}

This will guarantee you don't block ThreadPool threads.

Zer0
  • 7,191
  • 1
  • 20
  • 34
  • for those of us interested to see how this works - https://referencesource.microsoft.com/#mscorlib/system/Collections/Concurrent/ConcurrentQueue.cs,18bcbcbdddbcfdcb. Atomic increment and spinlock. Which BTW is how c# lock works anyway - see http://stackoverflow.com/questions/5111779/lock-monitor-internal-implementation-in-net – pm100 Nov 23 '16 at 00:02
  • 1
    @pm100 - That's quite incorrect. The `lock` keyword is synonymous with `Monitor.Enter` and `Monitor.Exit` both of which can cause a context switch and blocking. Yes they do attempt to spin and they do use interlocked. But they can cause a context switch and put the thread in a blocked (waiting) state. This solution does not. Avoiding (relatively) very expensive context switches is a large reason why lock-free algorithms are used. Not to mention the transition from user mode to kernel mode and various other things like deadlocks. – Zer0 Nov 23 '16 at 00:09
  • did you read the article, says that lock is implemented in user mode code by the CLR if there is not contention - only if contention does it do kernel switch. Which will be the >90% case – pm100 Nov 23 '16 at 00:15
  • @pm100 I'm extremely well aware how it all works. No need to read it (and your link does mention blocking, fyi). OP boldly pointed out his reluctance to blocking (_especially_ the `ThreadPool`) and I don't blame him. So I proposed a simple non-blocking solution. Problem solved. – Zer0 Nov 23 '16 at 01:04
  • @Zer0 Thanks, looks promising - I will check it out and let you guys know how it does – Bem Nov 23 '16 at 14:19
0

Any kind of collection that is thread safe/synchronized will do. There are plenty in .net framework.

You can also use volatile variables to store data between multiple threads - but this is usually considered a bad practice.

Another approach can be to schedule those operations on tasks that can produce results, they run by default on the thread pool and you can get the return values by awaiting the methods and checking the Result of the Task that is returned.

Finally you can write your own code in order to synchronize access to certain regions of code/variables etc using stuff like lock, semaphores, mutex etc

Claudiu Cojocaru
  • 37
  • 5
  • 1
    Beware the use of `volatile`. It certainly does not guarantee thread-safety. – Zer0 Nov 22 '16 at 23:44
  • Yes, I agree, using volatile is usually a bad practice that's why I recommended using lock/semaphores/mutex as alternatives. I'll edit my reply and remove the volatile suggestion. – Claudiu Cojocaru Nov 22 '16 at 23:49
  • @ClaudiuCojocaru thanks for help, but I wrote I would like to avoid locking, blocking, semaphores etc. which are very bad inside threadpool. – Bem Nov 23 '16 at 14:21