Running multiple Threads sequentially?

Question

Maybe my approach is wrong, feel free to correct me.

I have the following method on my service:

public void PublishPriceChange()
{
     for (int i = 0; i < 200; i++)
     {
          Task.Run(() =>
          {
               int j = i;
               lock (_locker)
               {
                  TransformPrices(_quotes);
                  PriceChangeEventArgs e = new PriceChangeEventArgs(_quotes);
                  PriceChanged(this, e);
               }
           });                
     }            
}

_quotes contains the opening prices of the day and TransformPrices() simply changes these prices slightly upon each call. This simulates the changes during a day. Hence these tasks have to be run in a sequential way.

However having them running like this on a thread pool means each task is running in parallel and no longer in a sequence.

Obviously I could just run them without multi threading to get the required effect. However since the calculation takes some time, it would be good to keep the service responsive while its doing these tasks in the background.

Maybe I could use Continuations (awaiter) but not sure how to chain this all way through. Any advice?

Why do you need to run each instance in separate thread? You can create one worker thread which process prices. For example you want to process prices each hour, than in your worker thread you can call Thread.Sleep(oneHour) and each hour your TransformPrices method will occur. — Yevgeniy.Chernobrivets, Feb 19 '14 at 11:52
If you need to run these things sequentially then running them multi-threaded has no benefit anyway. What you're probably looking for is a *single * background thread that runs your entire loop. — Dan Puzey, Feb 19 '14 at 12:01

score 2 · Accepted Answer · edited May 23 '17 at 12:28

2

So, you want to transform the prices sequentially. Your goal seems to be to keep the service responsive (i.e., free the calling thread to do other work).

Why not use a task to run the whole loop?

public async Task PublishPriceChange()
{
    await Task.Run(() =>
    {
        lock (_locker)
        {
            for (int i = 0; i < 200; i++)
            {                  
                TransformPrices(_quotes);
                PriceChangeEventArgs e = new PriceChangeEventArgs(_quotes);
                PriceChanged(this, e);
            }
        }  
    });           
}

You might as well await the task, you probably don't want a fire-and-forget behaviour (which is not advisable for most scenarios).
If you were using the lock to ensure the loop would run sequentially, you can remove it.
Otherwise, if there's a chance that PublishPriceChange will be called a second time before the first call completes, move the lock outside the loop - by locking once instead of 200 times, you'll minimize the overhead caused by context switches.

edited May 23 '17 at 12:28

Community

1
1

answered Feb 19 '14 at 11:49

dcastro

66,540
21
145
155

1

In that case, you can probably remove the locking – Georg Feb 19 '14 at 11:49
1

@Georg that depends. Why was the lock put there in the first place? If it was put there to ensure *this* loop would run sequentially, then yes. If `_quotes` is a variable shared with other unrelated threads, then it should probably stay there. – dcastro Feb 19 '14 at 11:52
From the OP, I assumed that the lock was only in place to ensure that the inner code is executed sequentially. – Georg Feb 19 '14 at 11:55
Interesting discussion here. `TransformPrices()` writes to `_quotes`. Hence in my original post multiple threads could be writing to a shared state. But since now the whole loop is running on one separate thread, there is no concurrency anymore. Unless the whole loop is ran again, in which case i better make _quotes a local variable in first place? – Houman Feb 19 '14 at 12:00
@Georg That's a fair assumption - I've added a couple of notes to my answer. – dcastro Feb 19 '14 at 12:04
@Hooman Is there a chance `PublishPriceChange` will be called twice, before the first call has finished running? If so, since `_quotes` is not a method argument, I'm assuming it's a field of the enclosing type. In that case, it will be shared by both concurrent calls, and you should leave the lock there. – dcastro Feb 19 '14 at 12:10
1

@Hooman Actually - move the lock outside the loop. By locking once, instead of 200 times, you'll minimize the overhead caused by context switches. I'll update my code. – dcastro Feb 19 '14 at 12:12

Running multiple Threads sequentially?

1 Answers1