[RESOLVED] For each Next Loop

[billyboy630]

I am looking for some help in making this code better it works but it seems to be awfully slow. I am VB hobbyist and I suspect it is not written well.

Public Function GEOCode() As Boolean


        For Each address In GetNullAddressList()
            For i As Integer = 0 To dtlist.Rows.Count - 1

                Dim StreetAddress As String = CStr(dtlist.Rows(i)("FullStreetAddress"))
                Dim City As String = CStr(dtlist.Rows(i)("City"))
                Dim State As String = CStr(dtlist.Rows(i)("State"))
                Dim Zip As String = CStr(dtlist.Rows(i)("ZipCode"))
                SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                dtlist.Rows(i).Item("Latitude") = CStr(SearchAddress.Latitude)
                dtlist.Rows(i).Item("Longitude") = CStr(SearchAddress.Longitude)
            Next
            ProgressBar3.PerformStep()
        Next


        For Each address In GetNullAddressSold()
            For i As Integer = 0 To dtsold.Rows.Count - 1

                Dim StreetAddress As String = CStr(dtsold.Rows(i)("FullStreetAddress"))
                Dim City As String = CStr(dtsold.Rows(i)("City"))
                Dim State As String = CStr(dtsold.Rows(i)("State"))
                Dim Zip As String = CStr(dtsold.Rows(i)("ZipCode"))
                SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                dtsold.Rows(i).Item("Latitude") = CStr(SearchAddress.Latitude)
                dtsold.Rows(i).Item("Longitude") = CStr(SearchAddress.Longitude)
            Next
            ProgressBar3.PerformStep()
        Next


    End Function

[jmcilhinney]

Perhaps you could take the time to explain what the code is supposed to achieve, rather than making each of us work it out for ourselves. If we know what the point is, then we can determine what the best way to achieve that might be.

[billyboy630]

It loops through a data table and if a longitude or latitude value is missing it retrieves the value via a Google GEO COde API call. I saw an obvious mistake moments after posting my question.
This seems to work much better

For i As Integer = 0 To dtlist.Rows.Count - 1
            If IsDBNull(dtlist.Rows(i).Item("Latitude")) Or IsDBNull(dtlist.Rows(i).Item("Longitude")) Then
                Dim StreetAddress As String = CStr(dtlist.Rows(i)("FullStreetAddress"))
                Dim City As String = CStr(dtlist.Rows(i)("City"))
                Dim State As String = CStr(dtlist.Rows(i)("State"))
                Dim Zip As String = CStr(dtlist.Rows(i)("ZipCode"))
                SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                dtlist.Rows(i).Item("Latitude") = CStr(SearchAddress.Latitude)
                dtlist.Rows(i).Item("Longitude") = CStr(SearchAddress.Longitude)

                ProgressBar3.PerformStep()
            End If

        Next


        For i As Integer = 0 To dtsold.Rows.Count - 1
            If IsDBNull(dtsold.Rows(i).Item("Latitude")) Or IsDBNull(dtsold.Rows(i).Item("Longitude")) Then
                Dim StreetAddress As String = CStr(dtsold.Rows(i)("FullStreetAddress"))
                Dim City As String = CStr(dtsold.Rows(i)("City"))
                Dim State As String = CStr(dtsold.Rows(i)("State"))
                Dim Zip As String = CStr(dtsold.Rows(i)("ZipCode"))
                SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                dtsold.Rows(i).Item("Latitude") = CStr(SearchAddress.Latitude)
                dtsold.Rows(i).Item("Longitude") = CStr(SearchAddress.Longitude)

                ProgressBar3.PerformStep()

            End If

        Next

[jmcilhinney]

I don't see what the point of the For Each loops is. You never use the loop control variables, i.e. address, inside those loops so what are they for? Inside each For Each loop, you do the exact same thing ever time so you're just repeating yourself for no reason. As far as I can tell, you can just get rid of the outer loops and execute the inner loops once each to get the same result.

Also, inside the For loops, you keep using the loop counter over and over to get the same row every time. At the very least you should be just using the loop counter once to get the row and assigning it to a variable, then using that variable over and over. Better still, use a For Each loop over the Rows of the table.

Assuming that there's no issue with GetAddress being executed multiple times simultaneously, you could speed things up further by using Parallel.ForEach to process multiple rows on different threads at the same time.

EDIT: I was typing this while you were submitting post #3, so you've already addressed some of what I said.

[billyboy630]

Thanks for the help. It is actually looping through two separate Data Tables. The "For each" line in my original code posted was redundant as you pointed out.
I will look into the Parallel For each as things will get slow when there are large data tables. How does that compare to the background worker?

[jmcilhinney]

For Each row In dtList.Rows.Cast(Of DataRow)().Where(Function(dr) dr.IsNull("Latitude") OrElse dr.IsNull("Longitude"))
    Dim fullStreetAddress = row.Field(Of String)("FullStreetAddress")
    '...
Next

If you prefer without LINQ:

For Each row As DataRow In dtList.Rows
    If row.IsNull("Latitude") OrElse row.IsNull("Longitude")) Then
        Dim fullStreetAddress = CStr(row("FullStreetAddress"))
        '...
    End If
Next

I can't say that I'm a fan of storing the lat and long as text either.

[jmcilhinney]

I will look into the Parallel For each as things will get slow when there are large data tables. How does that compare to the background worker?

They both use thread pool threads to do background work. The BackgroundWorker uses one background thread, ostensibly to allow long-running work to be performed while the UI remains responsive. The Parallel.ForEach method uses multiple background threads to perform the same task on multiple list items simultaneously, ostensibly to reduce the overall time to process a list. If you want to process multiple list items simultaneously while maintaining a responsive UI then you might want to use both together. If you call Parallel.ForEach on the UI thread then you will block it, although not for as long as a standard For Each loop. If you call it in the DoWork event handler of a BackgroundWorker then you maintain a responsive UI while taking less time to process a list of items.

[billyboy630]

Thank you

p.s. no sure why I was storing the LAT and LONG as strings I needed them as integers later
Thanks for pointing that out

[billyboy630]

They both use thread pool threads to do background work. The BackgroundWorker uses one background thread, ostensibly to allow long-running work to be performed while the UI remains responsive. The Parallel.ForEach method uses multiple background threads to perform the same task on multiple list items simultaneously, ostensibly to reduce the overall time to process a list. If you want to process multiple list items simultaneously while maintaining a responsive UI then you might want to use both together. If you call Parallel.ForEach on the UI thread then you will block it, although not for as long as a standard For Each loop. If you call it in the DoWork event handler of a BackgroundWorker then you maintain a responsive UI while taking less time to process a list of items.

I followed the non-LINQ code and I placed it in a background worker do work sub. It is working but starts fast then slows down. I went back to re-read a comment you made about GetAddress being executed multiple times simultaneously. I am wondering if that is till case I can't how? GetAddress is a function that uses the Google geo code Api to return latitude and longitude

I am also trying to implement the Parallel.ForEach you suggested and struggling her is what I have so far

Parallel.ForEach(row As datarow In dtlist.AsEnumerable(DataRow), Sub(row) , (sqirly red under the whole line so not even close LOL

For Each row As DataRow In dtlist.Rows

            If row.IsNull("Latitude") OrElse row.IsNull("Longitude") Then
                Dim StreetAddress = CStr(row("FullStreetAddress"))
                Dim City As String = CStr(row("City"))
                Dim State As String = CStr(row("State"))
                Dim Zip As String = CStr(row("ZipCode"))
                SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                row.Item("Latitude") = (SearchAddress.Latitude)
                row.Item("Longitude") = (SearchAddress.Longitude)

            End If
        Next

        For Each roww As DataRow In dtsold.Rows
            If roww.IsNull("Latitude") OrElse roww.IsNull("Longitude") Then
                Dim StreetAddress = CStr(roww("FullStreetAddress"))
                Dim City As String = CStr(roww("City"))
                Dim State As String = CStr(roww("State"))
                Dim Zip As String = CStr(roww("ZipCode"))
                SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                roww.Item("Latitude") = (SearchAddress.Latitude)
                roww.Item("Longitude") = (SearchAddress.Longitude)
            End If

        Next

[jmcilhinney]

Parallel.ForEach(myDataTable.AsEnumerable(),
                 Sub(row)
                     If row.IsNull("Latitude") OrElse row.IsNull("Longitude") Then
                         '...
                     End If
                 End Sub)

[billyboy630]

Do I follow the same for both For Each statements?

[jmcilhinney]

Do I follow the same for both For Each statements?

If a call to Parallel.ForEach replaces a For Each loop and you have two For Each loops, what do you think you need to do? Like I said though, Parallel.Foreach itself is a synchronous method, so you're still going to fully process one DataTable before starting to process the other. If that's an issue then you still need to change things to execute both Parallel.ForEach calls simultaneously. One step at a time though.

[billyboy630]

Gotcha
How would you recommend reporting progress when using the Parallel.ForEach?

I was doing it this way

BackgroundWorker3.ReportProgress(i, i & " " & "Records Processed")

but not sure now that i don't have the i integer loop

[jmcilhinney]

Maybe spend some time thinking about it for yourself and then ask for help if you get stuck. As a hint, consider the fact that, every time a record is processed, you know that you have now processed one more record than you had before.

[billyboy630]

Trust me by the time I post something here I have thought about it considerably and google searched. So if I post a question here it is because I am stuck. Posting a question is NOT my first method of problem solving

Here was my first thought since each row represents one more record processed

BackgroundWorker3.ReportProgress(row, row & " " & "Records Processed")
error is operator & is not defined for types datarow and string

[jmcilhinney]

Trust me by the time I post something here I have thought about it considerably and google searched. So if I post a question here it is because I am stuck. Posting a question is NOT my first method of problem solving

Here was my first thought since each row represents one more record processed

BackgroundWorker3.ReportProgress(row, row & " " & "Records Processed")
error is operator & is not defined for types datarow and string

I'm afraid that I find that hard to believe. I provided the example code for Parallel.ForEach two hours ago and your best attempt shows that you don't actually know the data type of that row parameter. It seems unlikely that a significant portion of that two hours has been spent considering how to track progress. You certainly didn't spend any significant time considering the hint I provided. Why does the background thread have to tell the UI thread anything, other than that another row has been processed? You know for a fact that zero rows have been processed to start with and you know for a fact that, each time the ProgressChanged event is raised, one more row has been processed. Why is that not enough information for the UI thread to track progress? If a standard message containing that number needs to be displayed, why would that message need to come from the background thread?

[billyboy630]

You confuse an individual's actual ability with your expectations of what you think their ability should be. Believe what you want but I have been working on this all-day (literally). You do this fulltime for years. This is a hobby that I have not worked on for years. While I appreciate your wanting to guide people into learning on their own as opposed to just posting questions, let's try and keep the condescension to a minimum.

"MY" understanding is that the BGM progress change would update the progress bar so i have

ProgressBar3.Value = e.ProgressPercentage
Label1.Text = TryCast(e.UserState, String)

My understanding also that you need to invoke the BGW Report Progress in the BGW Dowork?

That is how I had it when I was using the I as integer loop and I passed i as a counter and number for my label

[jmcilhinney]

Like many people, you fail to separate logic from code. You should be determining the logic involved first and then writing code to implement that logic. We all use logic every day so there's often no need to have any programming experience at all to work out the logic. Consider this scenario. Let's say that I contract you to build a house and you subcontract others do the bricklaying, the carpentry, the plumbing, etc. At the start of the job, I know how many individual tasks there will be. As each subcontractor finishes their task, they come to me and let me know. Does each subcontractor have to tell me how many tasks have been completed? Of course they don't. They just have to tell me that they finished their one task. I add 1 to my running count of completed tasks and, when that count is equal to the total number of tasks then I know that the house is finished. I doubt that that confuses you at all as it's simple logic. Your situation is analogous. ReportProgress, ProgressChanged, add 1.

[billyboy630]

I understand the logic I don't understand how to get the variable (if that is the correct word) to get the progress

In my previous way I could see that "i" was the next step that was completed that is why I thought row would be the equivalent but row can not be converted to an integer

BackgroundWorker3.ReportProgress(row)

[jmcilhinney]

I don't understand how to get the variable

What variable? There is no variable to get. In the ProgressChanged event handler you simply need to increment the count by 1. What variable do you need to do that? You don't. You already have the count of processed rows in the Value of the ProgressBar. It starts at zero and you increment it by 1 each time the event is raised. ReportProgress requires to you pass an Integer but that Integer is going to be ignored in the ProgressChanged event handler so it doesn't matter what it is. The most logical choice is Nothing, which is interpreted as zero for type Integer, but you could use 0, 1, 999, Integer.MinValue, Integer.MaxValue or any other Integer. It's irrelevant. Stop trying to fit what you already have into the new scenario and start thinking about what you need for the new scenario. As I have already said previously, the UI thread doesn't need any information from the background thread other than the fact that a row has been processed, so stop trying to work out how to get information that is not needed.

[billyboy630]

So here is what I arrived at

This is in my Parallel.For Each
BackgroundWorker3.ReportProgress(0, "Records Retrieved")

This is in BGW Progress Changed
Label1.Visible = True
Label1.Text = TryCast(e.UserState, String)
ProgressBar3.Value += 1

I get an error at end because the value is adding one more than the Max Value of Progress Bar I am setting the MAX value for progress bar with null rows in the DataTable. The problem I think is the +=1 is adding one more than is in the DataTable

I would still like to pass the user state so my label can have the number of records of that are processed

[jmcilhinney]

Show us all the relevant code, i.e. the setup before calling RunWorkerAsync, the DoWork event handler and the ProgressChanged event handler. My suspicion regarding the ProgressBar is that you have set MaxValue to the number of rows containing NULLs but then you're incrementing the Value for every row, but that's just a guess without seeing all the code. I'll post an example of what I envision it should be.

[billyboy630]

That was an excellent guess. I was setting the Max value to number of rows with Null value while in the code I was checking each row for null value. Thank you

Private Sub Button1_Click_1(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button1.Click
        If Me.BackgroundWorker1.IsBusy Then Exit Sub

        Dim ListingBadRecords As Double = (GetNullAddressList.Count)
        Dim SoldBadRecords As Double = (GetNullAddressSold.Count)
        Dim totalbadrecords As Double = ListingBadRecords + SoldBadRecords

        If ListingBadRecords > 0 Then

            Dim result1 As DialogResult = MessageBox.Show("There Are" & " " & ListingBadRecords & " " & "Listing Properties Missing GeoCode Coordinates" & " " & vbCrLf & "Click Ok to Get the Data or Cancel to Quit", "Missing Data", MessageBoxButtons.OKCancel, MessageBoxIcon.Error)
            If result1 = DialogResult.OK Then

                Try
                    Label1.Visible = True
                    ProgressBar3.Visible = True
                    ProgressBar3.Minimum = 0
                    ProgressBar3.Maximum = CInt(dtlist.Rows.Count)
                    BackgroundWorker3.RunWorkerAsync()
                Catch ex As Exception
                    MsgBox(ex.Message)
                End Try
            End If
        End If

        If SoldBadRecords > 0 Then
            Dim result2 As DialogResult = MessageBox.Show("There Are" & " " & SoldBadRecords & " " & "Sold Properties Missing GeoCode Coordinates" & " " & vbCrLf & "Click Ok to Get the Data or Cancel to Quit", "Missing Data", MessageBoxButtons.OKCancel, MessageBoxIcon.Error)
            If result2 = DialogResult.OK Then

                Try
                    Label1.Visible = True
                    ProgressBar3.Visible = True
                    ProgressBar3.Minimum = 0
                    ProgressBar3.Maximum = CInt(dtsold.Rows.Count)
                    BackgroundWorker4.RunWorkerAsync()
                Catch ex As Exception
                    MsgBox(ex.Message)
                End Try

            End If
        End If

    End Sub

Private Sub BackgroundWorker3_DoWork(ByVal sender As Object, ByVal e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker3.DoWork
        'gets GEO codes for Listings
        Parallel.ForEach(dtlist.AsEnumerable(),
                         Sub(row)
                             If row.IsNull("Latitude") OrElse row.IsNull("Longitude") Then
                                 Dim StreetAddress = CStr(row("FullStreetAddress"))
                                 Dim City As String = CStr(row("City"))
                                 Dim State As String = CStr(row("State"))
                                 Dim Zip As String = CStr(row("ZipCode"))
                                 SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                                 row.Item("Latitude") = (SearchAddress.Latitude)
                                 row.Item("Longitude") = (SearchAddress.Longitude)
                             End If
                             BackgroundWorker3.ReportProgress(0, "Records Retrieved")

                         End Sub)


    End Sub

Private Sub BackgroundWorker3_ProgressChanged(ByVal sender As Object,
                                              ByVal e As System.ComponentModel.ProgressChangedEventArgs) Handles BackgroundWorker3.ProgressChanged

        Label1.Visible = True
        Label1.Text = TryCast(e.UserState, String)
        ProgressBar3.Value += 1 '= e.ProgressPercentage
    End Sub

[billyboy630]

I came up with this a a solution. Not sure its the best way to do it but I can't see why not

 Private Sub BackgroundWorker4_DoWork(ByVal sender As Object, ByVal e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker4.DoWork
        'gets GEO codes for Solds
        Dim i As Integer = 0
        Parallel.ForEach(dtsold.AsEnumerable(),
                         Sub(row)
                             If row.IsNull("Latitude") OrElse row.IsNull("Longitude") Then
                                 Dim StreetAddress = CStr(row("FullStreetAddress"))
                                 Dim City As String = CStr(row("City"))
                                 Dim State As String = CStr(row("State"))
                                 Dim Zip As String = CStr(row("ZipCode"))
                                 SearchAddress = GetAddress(StreetAddress, City, State, Zip)
                                 row.Item("Latitude") = (SearchAddress.Latitude)
                                 row.Item("Longitude") = (SearchAddress.Longitude)
                                 i += 1
                             End If
                             BackgroundWorker4.ReportProgress(i, i & " " & "Records Retrieved")

                         End Sub)


    End Sub

[jmcilhinney]

Did that work for you? I've been basing my advice on theory so far and now that I've tested something I get ArgumentOutOfRangeExceptions and I'm not sure why. From what i can see so far, it looks like editing a row causes indexes within the collection to change and so editing multiple rows at the same time causes confusion regarding which row relates to which index. I'll look into it a bit further but I'm interested to know whether you're seeing anything similar.

[billyboy630]

I have not tested on a large DataTable but ran it on a 50 record one and it ran flawlessly
Perhaps on a larger Table it might cause confusion? I will run it and let you know

[jmcilhinney]

If I do this, it works as expected:

Imports System.ComponentModel
Imports System.Threading
 
Public Class Form1
 
    Private ReadOnly rng As New Random
    Private table1 As DataTable
    Private table2 As DataTable
 
    Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
        table1 = GetTable()
        table2 = GetTable()
 
        ProgressBar1.Value = 0
        ProgressBar1.Maximum = table1.Rows.Count + table2.Rows.Count
 
        Button1.Enabled = False
 
        BackgroundWorker1.RunWorkerAsync()
    End Sub
 
    Private Sub BackgroundWorker1_DoWork(sender As Object, e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker1.DoWork
        Try
            Parallel.ForEach(table1.AsEnumerable(),
                             Sub(row As DataRow)
                                 If row.Field(Of Boolean)("Flag") Then
                                     Thread.Sleep(200)
                                     'row("Flag") = False
                                 End If
 
                                 BackgroundWorker1.ReportProgress(0)
                             End Sub)
 
            Parallel.ForEach(table2.AsEnumerable(),
                             Sub(row As DataRow)
                                 If row.Field(Of Boolean)("Flag") Then
                                     Thread.Sleep(200)
                                     'row("Flag") = False
                                 End If
 
                                 BackgroundWorker1.ReportProgress(0)
                             End Sub)
        Catch ex As Exception
            Console.WriteLine(ex.ToString())
        End Try
    End Sub
 
    Private Sub BackgroundWorker1_ProgressChanged(sender As Object, e As ProgressChangedEventArgs) Handles BackgroundWorker1.ProgressChanged
        Dim recordCount = ProgressBar1.Value
 
        recordCount += 1
        Label1.Text = $"{recordCount} records processed"
        ProgressBar1.Value = recordCount
    End Sub
 
    Private Sub BackgroundWorker1_RunWorkerCompleted(sender As Object, e As RunWorkerCompletedEventArgs) Handles BackgroundWorker1.RunWorkerCompleted
        Label1.Text = "All records processed"
        Button1.Enabled = True
    End Sub
 
    'Creates a DataTable with a random number of rows between 1000 and 2000 with an auto-incremented Id column and a Flag column of random Booleans.
    Private Function GetTable() As DataTable
        Dim table As New DataTable
 
        With table.Columns
            .Add("Id", GetType(Integer)).AutoIncrement = True
            .Add("Flag", GetType(Boolean))
        End With
 
        For i = 0 To rng.Next(1000, 2000)
            Dim row = table.NewRow()
 
            row("Flag") = (rng.Next() Mod 2 = 0)
            table.Rows.Add(row)
        Next
 
        Return table
    End Function
 
End Class

If I uncomment the lines that edit the rows though, I see those exceptions thrown:

Private Sub BackgroundWorker1_DoWork(sender As Object, e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker1.DoWork
    Try
        Parallel.ForEach(table1.AsEnumerable(),
                         Sub(row As DataRow)
                             If row.Field(Of Boolean)("Flag") Then
                                 Thread.Sleep(200)
                                 row("Flag") = False
                             End If
 
                             BackgroundWorker1.ReportProgress(0)
                         End Sub)
 
        Parallel.ForEach(table2.AsEnumerable(),
                         Sub(row As DataRow)
                             If row.Field(Of Boolean)("Flag") Then
                                 Thread.Sleep(200)
                                 row("Flag") = False
                             End If
 
                             BackgroundWorker1.ReportProgress(0)
                         End Sub)
    Catch ex As Exception
        Console.WriteLine(ex.ToString())
    End Try
End Sub

If I lock that line to ensure only one row gets edited at a time, it works without issue:

Private Sub BackgroundWorker1_DoWork(sender As Object, e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker1.DoWork
    Try
        Parallel.ForEach(table1.AsEnumerable(),
                         Sub(row As DataRow)
                             If row.Field(Of Boolean)("Flag") Then
                                 Thread.Sleep(200)
 
                                 SyncLock table1
                                     row("Flag") = False
                                 End SyncLock
                             End If
 
                             BackgroundWorker1.ReportProgress(0)
                         End Sub)
 
        Parallel.ForEach(table2.AsEnumerable(),
                         Sub(row As DataRow)
                             If row.Field(Of Boolean)("Flag") Then
                                 Thread.Sleep(200)
 
                                 SyncLock table2
                                     row("Flag") = False
                                 End SyncLock
                             End If
 
                             BackgroundWorker1.ReportProgress(0)
                         End Sub)
    Catch ex As Exception
        Console.WriteLine(ex.ToString())
    End Try
End Sub

That defeats the purpose somewhat, as none of the actual editing can occur in parallel, but the checking of the rows to process and the retrieving of the data could still be done in parallel in your case.

Note that this will include every row in the progress feedback. If you only want to include the rows that will actually be modified in the progress then you would need to calculate that number at the start and then you can change your Parallel.Foreach calls a little:

Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
    table1 = GetTable()
    table2 = GetTable()
 
    ProgressBar1.Value = 0
    ProgressBar1.Maximum = table1.AsEnumerable().Count(Function(row) row.Field(Of Boolean)("Flag")) +
                           table2.AsEnumerable().Count(Function(row) row.Field(Of Boolean)("Flag"))
 
    Button1.Enabled = False
 
    BackgroundWorker1.RunWorkerAsync()
End Sub
 
Private Sub BackgroundWorker1_DoWork(sender As Object, e As System.ComponentModel.DoWorkEventArgs) Handles BackgroundWorker1.DoWork
    Try
        Parallel.ForEach(table1.AsEnumerable().Where(Function(row) row.Field(Of Boolean)("Flag")),
                         Sub(row As DataRow)
                             Thread.Sleep(200)
 
                             SyncLock table1
                                 row("Flag") = False
                             End SyncLock
 
                             BackgroundWorker1.ReportProgress(0)
                         End Sub)
 
        Parallel.ForEach(table2.AsEnumerable().Where(Function(row) row.Field(Of Boolean)("Flag")),
                         Sub(row As DataRow)
                             Thread.Sleep(200)
 
                             SyncLock table2
                                 row("Flag") = False
                             End SyncLock
 
                             BackgroundWorker1.ReportProgress(0)
                         End Sub)
    Catch ex As Exception
        Console.WriteLine(ex.ToString())
    End Try
End Sub

[billyboy630]

I am doing one table at a time look at post #23
I added the i += 1 in the foreach statement so i had an integer to pass to get a record count into my label in my background worker progress changed

[jmcilhinney]

There's something interesting to note about that last code snippet. I removed the If statements and added a Where call to neaten things up a bit. I then realised that that means that each row will be tested to see whether it needs to be processed in serial rather than parallel. That would slow things down a bit so I was about to change it back. I then realised that it would also reduce the overhead of accessing rows on other threads that didn't actually need to be edited, as the editing is the slow part. You'd actually need to do some testing to determine which is going to be faster, but I'll leave that up to you. Just note that, if you do use If statements, you'd need to report progress from inside that If block:

Private Sub BackgroundWorker1_DoWork(sender As Object, e As DoWorkEventArgs) Handles BackgroundWorker1.DoWork
    Try
        Parallel.ForEach(table1.AsEnumerable(),
                         Sub(row As DataRow)
                             If row.Field(Of Boolean)("Flag") Then
                                 Thread.Sleep(200)
 
                                 SyncLock table2
                                     row("Flag") = False
                                 End SyncLock
 
                                 BackgroundWorker1.ReportProgress(0)
                             End If
                         End Sub)
 
        Parallel.ForEach(table2.AsEnumerable(),
                         Sub(row As DataRow)
                             If row.Field(Of Boolean)("Flag") Then
                                 Thread.Sleep(200)
 
                                 SyncLock table2
                                     row("Flag") = False
                                 End SyncLock
 
                                 BackgroundWorker1.ReportProgress(0)
                             End If
                         End Sub)
    Catch ex As Exception
        Console.WriteLine(ex.ToString())
    End Try
End Sub

[jmcilhinney]

I am doing one table at a time

I assume that that was just for testing purposes. You originally wanted to do both so I've provided an example that does two. The code only processes one table at a time though, despite processing both in a single run. Parallel.ForEach is itself synchronous, so it won't complete until each item in the list has been processed. Items within the list get processed in parallel though. That doesn;t make any difference to the exceptions I was getting though, as they always occurred on the first table.

I added the i += 1 in the foreach statement so i had an integer to pass to get a record count into my label in my background worker progress changed

I know you're doing that, despite that fact that I've stated, over and over, that it's pointless. The code I posted demonstrates why it's pointless. The UI thread knows how many records have been processed so it doesn't need that background thread to tell it.

[billyboy630]

I will run it with larger datable tomorrow and see if i get exceptions

I know you said the UI thread knows how many records have been processed I struggled obviously to figure out how to get the count into the label

Dim recordCount = ProgressBar1.Value

recordCount += 1
Label1.Text = $"{recordCount} records processed"

This was such an obvious solution I am embarrassed lol

[jmcilhinney]

Actually, you don't need to make two calls to Parallel.ForEach. Because the code is the same both tables, you can combine the rows from each into a single list and process it with a single call:

Parallel.ForEach(table1.AsEnumerable().Concat(table2.AsEnumerable()),
                 Sub(row As DataRow)
                     If row.Field(Of Boolean)("Flag") Then
                         Thread.Sleep(200)
 
                         SyncLock row.Table
                             row("Flag") = False
                         End SyncLock
 
                         BackgroundWorker1.ReportProgress(0)
                     End If
                 End Sub)

There's also the option of creating your own Tasks:

Task.WaitAll((From row In table1.AsEnumerable().Concat(table2.AsEnumerable())
              Select Task.Run(Sub()
                                  If row.Field(Of Boolean)("Flag") Then
                                      Thread.Sleep(200)
 
                                      SyncLock row.Table
                                          row("Flag") = False
                                      End SyncLock
 
                                      BackgroundWorker1.ReportProgress(0)
                                  End If
                              End Sub)).ToArray())

I did some testing with 1000 rows in each table and all flags set to True. I did three runs for each of the three options and I got the following average times:

2 x Parallel.ForEach: 30.25 seconds
1 x Parallel.ForEach: 34.4 seconds
Task.Run: 26.52 seconds

Some further testing might be required to confirm those numbers but that seems to suggest which option is better.