0

THERE'S AN UPDATE BELOW THIS INITIAL QUESTION

I have a query that pulls in about 90,000 header records. I then want to iterate through that result set to get detail data for each header retrieved. If I process it linearly it take close to an hour and a half. If I parallelize it, I can get it done in 11 minutes. However, there's no screen updates. I have done multi-threaded applications lots of times, and have always been successful with doing things like:

this.lblStatus.Invoke(new MethodInvoker(() => this.lblStatus.Text = "Updating Standards Docs"));

However, this appears to really screen up a Parallel loop. Using that method for some screen updates, the Parallel loop never actually finished. So I need another method.

I've been trying:

Task.Factory.StartNew(() =>
            {
                OrderablePartitioner<PayrollHeader> partitioner = Partitioner.Create(query, EnumerablePartitionerOptions.NoBuffering);
                Parallel.ForEach(partitioner, thisCheck =>
                {
                    Interlocked.Increment(ref iChckNo);

                    lock (_status)
                    {
                        _status.ProcessMsg = "Voucher " + thisCheck.VoucherNumber;
                        _status.ProcessName = thisCheck.EmployeeName;
                        _status.CurrentRec = iChckNo;
                        dtSpan = DateTime.Now.Subtract(dtSpanStart);
                        _status.TimeMsg = string.Format("Elapsed {0}:{1}:{2}", dtSpan.Hours, dtSpan.Minutes, dtSpan.Seconds);
                    }

                    BeginInvoke((Action) (() =>
                    {
                        lblVoucher.Text = _status.ProcessMsg;
                        lblName.Text = _status.ProcessName;
                        lblCount.Text = string.Format("Record {0} of {1}", _status.CurrentRec, _status.TotalRecs);
                        lblTime.Text = _status.TimeMsg;
                        Application.DoEvents();
                    }));

                    thisCheck.GetDetails();
                });
            }).Wait();

The wait on the Task is because afterwards I need to do something else with the query afterwards, which I'll put into a ContinueWith statement eventually, I just really need to get the screen update to work.

I know all about cross thread corruption, which is why I'm trying to use the Invoker method... I firmly believe long running processes still need to keep the user informed, which is why I'm attempting this.

BTW, it's a WinForms app, not a WPF app. Any help at all would be greatly appreciated...

UPDATE:So someone wanted to see the updated code, with the IProgress into it.

Status _status = new Status {TotalRecs = query.Count};

            var progress = new Progress<Status>(msg => WriteStatusUpdate(msg));

            Task.Run(() =>
            {
                OrderablePartitioner<PayrollHeader> partitioner = Partitioner.Create(query, EnumerablePartitionerOptions.NoBuffering);
                Parallel.ForEach(partitioner, thisCheck =>
                {
                    lock (_status)
                    {
                        _status.ProcessMsg = "Voucher " + thisCheck.VoucherNumber;
                        _status.ProcessName = thisCheck.EmployeeName;
                        _status.CurrentRec = ++iChckNo;
                        dtSpan = DateTime.Now.Subtract(dtSpanStart);
                        _status.TimeMsg = string.Format("Elapsed {0}:{1}:{2}", dtSpan.Hours, dtSpan.Minutes, dtSpan.Seconds);
                    }

                    ((IProgress<Status>) progress).Report(_status);
                    thisCheck.GetDetails();
                });
            }).Wait();

 private void WriteStatusUpdate(Status _status)
    {
        lblVoucher.Text = _status.ProcessMsg;
        lblVoucher.Refresh();

        lblName.Text = _status.ProcessName;
        lblName.Refresh();

        lblCount.Text = string.Format("Records {0} of {1}", _status.CurrentRec, _status.TotalRecs);
        lblCount.Refresh();

        lblTime.Text = _status.TimeMsg;
        lblTime.Refresh();
    }

The code to update the screen never gets called...

Wizaerd
  • 235
  • 3
  • 15
  • Why did you use Parallel? Why not async programming? – Oscar Ortiz Jul 05 '16 at 17:10
  • Because I want to use a Parallel loop. Async doesn't offer anything better for this type of processing, and still has issues with screen updates. Plus I want multiple threads. – Wizaerd Jul 05 '16 at 17:16
  • What does Application.DoEvents() do? You can do multi-threading while each CPU do Async on its own.. just asking... so is that type of processing you mention 100% CPU? – Oscar Ortiz Jul 05 '16 at 17:21
  • Application.DoEvents give the UI thread time to process it's own messages, thus enabling screen refreshes to become apparent immediately. Parallel processing automagically doles out to multiple processors, and handles thread instantiation and destruction. My CPU does not pegs out at 100% because I 8 processors, and Parallel loops are optimized for multiple CPUs. But all this doesn't answer my question at all. Async wouldn't be any better in this instance. Doing this without a DoEvents changes nothing, and is pretty necessary if you want the control updates to be immediate. – Wizaerd Jul 05 '16 at 17:25
  • You are calling `DoEvents` a whole bunch of times, which may be an issue. According to [this](http://stackoverflow.com/a/570569/5095502) you should try `lblVoucher.Refresh()`, `lblName.Refresh()`, etc.. after updating their `.Text` values. – Quantic Jul 05 '16 at 17:27
  • For the sake of argument, I changed it, and the results don;t change. – Wizaerd Jul 05 '16 at 17:41
  • In all your examples (but especially on your IProgress version) Why do you use a class level `_status` variable? You should create a new `Status` inside of the parallel code section, this also lets you get rid of the `lock`. Also the fact that `thisCheck.GetDetails();` is at the end of the function seems odd, what is the GetDetails function doing? – Scott Chamberlain Jul 05 '16 at 20:48
  • There's no reason to keep creating a new Status class with every iteration of the loop, can just use the same instance. thisCheck.GetDetails is getting all the details for the paycheck voucher... Earnings, Deductions, Garnishments, etc... It's the work horse that is the long running process per check. – Wizaerd Jul 05 '16 at 21:06

2 Answers2

1

Don't try to update the UI from inside the parallel loop. It's not just that you can't update the UI from inside a background thread, it results in ugly and unmaintainable code. The parallel loop should do processing. Reporting should be performed by someone else.

The .NET Framework provides the IProgress< T> interface to report progress and the default implementation Progress< T> raises an event or calls a delegate on its creator thread, eg the UI thread. This results in much simpler code, eg:

var stopwatch = Stopwatch.StartNew();
var progressImpl=new Progress<Tuple<int,string,string>>(
                     msg=>ReportProgress(msg,stopwatch))
IProgress<Tuple<int,string,string>> progress=progressImpl;

var  partitioner = Partitioner.Create(query, EnumerablePartitionerOptions.NoBuffering);
Task.Run(()=> Parallel.ForEach(partitioner, thisCheck =>
            {
             .... 
             var msg=Tuple.Create(iChckNo,thisCheck.VoucherNumber,thisCheck.EmployeeName);
             progress.Report(msg);
             ...
            })
        );

 ...
private void ReportProgress(Tuple<int,string,string> msg,Stopwatch stopwatch)
{
    _status.ProcessMsg = "Voucher " + msg.Item2;
    _status.ProcessName = msg.Item3;
    _status.CurrentRec = msg.Item1;
    _status.TimeMsg = string.Format("Elapsed {0:c}", stopwatch.Elapsed);
};

I'm being very lazy here by using a Tuple<int,string,string> instead of a more specific class.

Messages sent from inside the parallel loop will be marshaled on the UI thread by Progress<T> and the ReportProgress function will be called on the UI thread itself.

The cast to IProgress< T> is necessary because the Publish method is explicitly implemented. This is a safety measure to prevent programmers from coding against the implementation itself.

Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
  • I tried doing this, and none of the screen updates ever show. In fact, putting a breakpoint and the method that is attempting to update the screen never even gets called... – Wizaerd Jul 05 '16 at 17:40
  • @Wizaerd then you have some other bug in your code. I would reccomend updating your question showing what you tried with `Progress` so we can help you point out where the mistake was. – Scott Chamberlain Jul 05 '16 at 17:57
  • When you say update the question, do you mean overwwrite the initial question I wrote, or edit it by adding a new Answer? – Wizaerd Jul 05 '16 at 18:14
0

I couldn't get the IProgress thing to actually work, so what I ended up doing, which is probably not the best approach is I put the Parallel.ForEach in a Task, and in the loop I update a public object. When the Task actually starts I sit in a while loop until it's done, and in that while loop I'm updating the UI...

bool blDone = false;
int iChckNo = 0;
_status.TotalRecs = query.Count;
Task.Run(() =>
{
    OrderablePartitioner<PayrollHeader> partitioner = Partitioner.Create(query, EnumerablePartitionerOptions.NoBuffering);
    Parallel.ForEach(partitioner, thisCheck =>
    {
        lock (_status)
        {
            iChckNo++;
            _status.ProcessMsg = "Voucher " + thisCheck.VoucherNumber;
            _status.ProcessName = thisCheck.EmployeeName;
            _status.CurrentRec = iChckNo;
            dtSpan = DateTime.Now.Subtract(dtSpanStart);
            _status.TimeMsg = string.Format("Elapsed {0}:{1}:{2}", dtSpan.Hours, dtSpan.Minutes, dtSpan.Seconds);
        }

        thisCheck.GetDetails();
    });

    blDone = true;
});

while (!blDone)
{
    WriteStatusUpdate();
}

further down in the code is

private void WriteStatusUpdate()
{
    lock (_status)
    {
        lblVoucher.Text = _status.ProcessMsg;
        lblName.Text = _status.ProcessName;
        lblCount.Text = string.Format("Records {0} of {1}", _status.CurrentRec, _status.TotalRecs);
        lblTime.Text = _status.TimeMsg;
        Application.DoEvents();
    }
}

Again, most likely not the best approach, but whatever gets it done...

Wizaerd
  • 235
  • 3
  • 15