Answered by:
foreach Loop Performance

Question
-
Hi,
I fetch the database to my code using SQL command and set in DataBase, I have foreach loop on this datatable it's take long time when the command return big number of the records.
any idea to enhance the performance.
Notes: - Language is C# and the application is web application.
- I used for loop but that's same.
foreach (DataRow r in table.Rows)
{
events.Add(new RptEventDetails
{
DriverId = Converter.ToInt32(r["Driverid"]),
VehicleId = Converter.ToInt32(r["VehicleId"]),
DriverName = r["DriverName"].ToString(),
VehicleName = r["VehicleName"].ToString(),
EventCode = Converter.ToInt32(r["EventCode"]),
EventName = r["EventName"].ToString(),
EventId = Converter.ToInt32(r["EventId"]),
StartDate = Converter.ToDateTime(r["EventStart"]) ?? DateTime.Now,
EndDate = Converter.ToDateTime(r["EventEnd"]) ?? DateTime.Now
});}
Thanks in advance.
- Edited by AhmadMou Wednesday, August 8, 2012 1:45 PM
Wednesday, August 8, 2012 1:40 PM
Answers
-
And to add to Fernando`s post, to improve performance, use Parse methods instead of ConvertTo:
int.Parse(yourValue) datetime.Parse(yourValue)
if you are sure there are those values. If you are not, better to use Convert, because in case (int.Parse()) will thrown an error, if you will try to convert string for example, when Convert.ToInt32() will only return 0.
About why your method need a lot of time to return, there is not such big difference between using string or index to get/set column, like Fernando explained there is a difference, but still not so much.
It seems you have a lot of rows inside database, thats why it takes so long, thats why I`d recommend you to put all this code on a new thread, and maybe use some progressBar control, to show to the user that there is something going on in the background.
Mitja
- Marked as answer by Lisa ZhuModerator Tuesday, August 21, 2012 10:19 AM
Wednesday, August 8, 2012 4:14 PM -
The most time consuming part is adding object to events collection. Instead of adding items one by one, use AddRange function to add objects in bulk,
RptEventDetails[] eventDetails= new RptEventDetails[table.Rows.Count); for(int i = 0; i < table.Rows.Count; i++) { DataRow r = table.Rows[i]; eventDetails[i] = new RptEventDetails { DriverId = Convert.ToInt32(r["Driverid"]), VehicleId = Convert.ToInt32(r["VehicleId"]), DriverName = Convert.ToString(r["DriverName"]), VehicleName = Convert.ToString(r["VehicleName"]), EventCode = Convert.ToInt32(r["EventCode"]), EventName = Convert.ToString(r["EventName"]), EventId = Convert.ToInt32(r["EventId"]), StartDate = r["EventStart"] == null? DateTime.Now : Converter.ToDateTime(r["EventStart"]); EndDate = r["EventEnd"] == null ? DateTime.Now : Convert.ToDateTime(r["EventEnd"]; }; }
Try this code.
events.AddRange(eventDetails);Please mark this post as answer if it solved your problem. Happy Programming!
- Marked as answer by Lisa ZhuModerator Tuesday, August 21, 2012 10:19 AM
Thursday, August 9, 2012 5:22 AM -
I suggest you use index rather than column name string while reading a field. To maintain readability while accessing column you can use enum.
I also suggest rather than using 'For each' loop, consider normal loop (for int i=0; i<table.Rows.count; i++) - should be useful when the collection is large.
- Marked as answer by Lisa ZhuModerator Tuesday, August 21, 2012 10:19 AM
Thursday, August 9, 2012 9:14 AM
All replies
-
-
Hi AhmadMou;
Accessing the columns of a DataRow object using the string name is great for readability and maintainability but this is done at the cost of performance. Each time you request to get or set the column by using the string name the system needs to look up the index of the item and then use that index to do the action needed. By using the index of the column you gain better performance at the cost of readability and maintainability. Try changing the string names and use the column index and see if it buys you any real performance.Fernando (MCSD)
If a post answers your question, please click "Mark As Answer" on that post and "Mark as Helpful".Wednesday, August 8, 2012 2:29 PM -
Depending on how many records you have you might use Parallel.ForEach or other asyncronous approach to read data rows from table and initialize RptEventDetails instances. Then again if you take this step you must remember that multiple threads can access the events.Add method at the same time. This might give you performance boost, but possibly with cost of more complex code.
Wednesday, August 8, 2012 3:03 PM -
And to add to Fernando`s post, to improve performance, use Parse methods instead of ConvertTo:
int.Parse(yourValue) datetime.Parse(yourValue)
if you are sure there are those values. If you are not, better to use Convert, because in case (int.Parse()) will thrown an error, if you will try to convert string for example, when Convert.ToInt32() will only return 0.
About why your method need a lot of time to return, there is not such big difference between using string or index to get/set column, like Fernando explained there is a difference, but still not so much.
It seems you have a lot of rows inside database, thats why it takes so long, thats why I`d recommend you to put all this code on a new thread, and maybe use some progressBar control, to show to the user that there is something going on in the background.
Mitja
- Marked as answer by Lisa ZhuModerator Tuesday, August 21, 2012 10:19 AM
Wednesday, August 8, 2012 4:14 PM -
The most time consuming part is adding object to events collection. Instead of adding items one by one, use AddRange function to add objects in bulk,
RptEventDetails[] eventDetails= new RptEventDetails[table.Rows.Count); for(int i = 0; i < table.Rows.Count; i++) { DataRow r = table.Rows[i]; eventDetails[i] = new RptEventDetails { DriverId = Convert.ToInt32(r["Driverid"]), VehicleId = Convert.ToInt32(r["VehicleId"]), DriverName = Convert.ToString(r["DriverName"]), VehicleName = Convert.ToString(r["VehicleName"]), EventCode = Convert.ToInt32(r["EventCode"]), EventName = Convert.ToString(r["EventName"]), EventId = Convert.ToInt32(r["EventId"]), StartDate = r["EventStart"] == null? DateTime.Now : Converter.ToDateTime(r["EventStart"]); EndDate = r["EventEnd"] == null ? DateTime.Now : Convert.ToDateTime(r["EventEnd"]; }; }
Try this code.
events.AddRange(eventDetails);Please mark this post as answer if it solved your problem. Happy Programming!
- Marked as answer by Lisa ZhuModerator Tuesday, August 21, 2012 10:19 AM
Thursday, August 9, 2012 5:22 AM -
I suggest you use index rather than column name string while reading a field. To maintain readability while accessing column you can use enum.
I also suggest rather than using 'For each' loop, consider normal loop (for int i=0; i<table.Rows.count; i++) - should be useful when the collection is large.
- Marked as answer by Lisa ZhuModerator Tuesday, August 21, 2012 10:19 AM
Thursday, August 9, 2012 9:14 AM -
I agree with Manu - classic loops seem faster than using an iterator. Find one of my posts that actually compares timing on foreach, for, and lambda, for looking for a character over 85K characters - the classic for was generally 15% faster
.Net thingy maker type guy.
- Proposed as answer by rachmann Friday, August 10, 2012 12:35 AM
Friday, August 10, 2012 12:34 AM