-
Jul 28th, 2020, 11:13 PM
#1
Thread Starter
Junior Member
[RESOLVED] For each Next Loop
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.
Code:
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
-
Jul 28th, 2020, 11:22 PM
#2
Re: [RESOLVED] For each Next Loop
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.
-
Jul 28th, 2020, 11:27 PM
#3
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
Code:
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
-
Jul 28th, 2020, 11:29 PM
#4
Re: [RESOLVED] For each Next Loop
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.
-
Jul 28th, 2020, 11:36 PM
#5
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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?
-
Jul 28th, 2020, 11:38 PM
#6
Re: [RESOLVED] For each Next Loop
vb.net Code:
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:
vb.net Code:
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.
Last edited by jmcilhinney; Jul 28th, 2020 at 11:59 PM.
-
Jul 28th, 2020, 11:43 PM
#7
Re: [RESOLVED] For each Next Loop
Originally Posted by billyboy630
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.
-
Jul 28th, 2020, 11:48 PM
#8
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Jul 29th, 2020, 06:51 PM
#9
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
Originally Posted by jmcilhinney
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
Code:
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
Last edited by billyboy630; Jul 29th, 2020 at 07:02 PM.
-
Jul 29th, 2020, 07:41 PM
#10
Re: [RESOLVED] For each Next Loop
vb.net Code:
Parallel.ForEach(myDataTable.AsEnumerable(),
Sub(row)
If row.IsNull("Latitude") OrElse row.IsNull("Longitude") Then
'...
End If
End Sub)
-
Jul 29th, 2020, 08:08 PM
#11
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
Do I follow the same for both For Each statements?
-
Jul 29th, 2020, 08:25 PM
#12
Re: [RESOLVED] For each Next Loop
Originally Posted by billyboy630
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.
-
Jul 29th, 2020, 09:12 PM
#13
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Jul 29th, 2020, 09:28 PM
#14
Re: [RESOLVED] For each Next Loop
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.
-
Jul 29th, 2020, 09:34 PM
#15
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Jul 29th, 2020, 09:55 PM
#16
Re: [RESOLVED] For each Next Loop
Originally Posted by 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
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?
-
Jul 29th, 2020, 10:40 PM
#17
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Jul 29th, 2020, 11:09 PM
#18
Re: [RESOLVED] For each Next Loop
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.
-
Jul 29th, 2020, 11:15 PM
#19
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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)
-
Jul 29th, 2020, 11:29 PM
#20
Re: [RESOLVED] For each Next Loop
Originally Posted by billyboy630
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.
-
Jul 30th, 2020, 11:11 AM
#21
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Jul 30th, 2020, 11:32 AM
#22
Re: [RESOLVED] For each Next Loop
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.
-
Jul 30th, 2020, 12:03 PM
#23
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
Code:
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
Code:
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
Code:
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
-
Jul 31st, 2020, 03:40 PM
#24
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
I came up with this a a solution. Not sure its the best way to do it but I can't see why not
Code:
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
-
Jul 31st, 2020, 11:44 PM
#25
Re: [RESOLVED] For each Next Loop
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.
-
Jul 31st, 2020, 11:48 PM
#26
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Aug 1st, 2020, 12:05 AM
#27
Re: [RESOLVED] For each Next Loop
If I do this, it works as expected:
vb.net Code:
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:
vb.net Code:
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:
vb.net Code:
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:
vb.net Code:
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
-
Aug 1st, 2020, 12:14 AM
#28
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Aug 1st, 2020, 12:16 AM
#29
Re: [RESOLVED] For each Next Loop
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:
vb.net Code:
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
-
Aug 1st, 2020, 12:21 AM
#30
Re: [RESOLVED] For each Next Loop
Originally Posted by billyboy630
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.
Originally Posted by billyboy630
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.
-
Aug 1st, 2020, 12:30 AM
#31
Thread Starter
Junior Member
Re: [RESOLVED] For each Next Loop
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
-
Aug 1st, 2020, 02:31 AM
#32
Re: [RESOLVED] For each Next Loop
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:
vb.net Code:
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:
vb.net Code:
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.
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|