19

What is the best way way to track progress in the following 

long total = Products.LongCount();
long current = 0;
double Progress = 0.0;

Parallel.ForEach(Products, product =>
{
    try
    {
        var price = GetPrice(SystemAccount, product);
        SavePrice(product,price);
    }
    finally
    {
        Interlocked.Decrement(ref this.current);
    }});

I want to update the progress variable from 0.0 to 1.0 (current/total) but i don't want to use anything that would have an adverse effect on the parallelism.

Madu Alikor
  • 2,544
  • 4
  • 21
  • 36
  • 1
    What is the progress being used for? Updating a UI? If so, could you get the UI to update itself (with a timer) based on the value of current and total? Doing too many updates per second on a UI can block it. Better to have the UI set the pace of updates rather than the processor. – Cameron MacFarland Jan 26 '13 at 14:07

3 Answers3

19

Jon's solution is good, if you need simple synchronization like this, your first attempt should almost always use lock. But if you measure that the locking slows things too much, you should think about using something like Interlocked.

In this case, I would use Interlocked.Increment to increment the current count, and change Progress into a property:

private long total;
private long current;
public double Progress
{
    get
    {
        if (total == 0)
            return 0;
        return (double)current / total;
    }
}

…

this.total = Products.LongCount();
this.current = 0;

Parallel.ForEach(Products, product =>
{
    try
    {
        var price = GetPrice(SystemAccount, product);
        SavePrice(product, price);
    }
    finally
    {
        Interlocked.Increment(ref this.current);
    }
});

Also, you might want to consider what to do with exceptions, I'm not sure that iterations that ended with an exception should be counted as done.

svick
  • 236,525
  • 50
  • 385
  • 514
5

Since you are just doing a few quick calculations, ensure atomicity by locking on an appropriate object:

long total = Products.LongCount();
long current = 0;
double Progress = 0.0;
var lockTarget = new object();

Parallel.ForEach(Products, product =>
{
    try
    {
        var price = GetPrice(SystemAccount, product);
        SavePrice(product,price);
    }
    finally
    {
        lock (lockTarget) {
            Progress = ++this.current / total;
        }
    }});
Jon
  • 428,835
  • 81
  • 738
  • 806
2

A solution without using any blocking in the body:

long total = Products.LongCount();
BlockingCollection<MyState> states = new BlockingCollection<MyState>();

Parallel.ForEach(Products, () =>
{
    MyState myState = new MyState();
    states.Add(myState);
    return myState;
},
(i, state, arg3, myState) =>
{
    try
    {
        var price = GetPrice(SystemAccount, product);
        SavePrice(product,price);
    }
    finally
    {
        myState.value++;
        return myState;
    }
},
i => { }
);

Then, to access the current progress:

(float)states.Sum(state => state.value) / total
Cédric Bignon
  • 12,892
  • 3
  • 39
  • 51