Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

So I started wondering why it takes 3.5 seconds to AcceptChanges for one changed record in a large DataTable. I wrote another implementation of AcceptChanges (VB.NET) - Me is a DataTable here:

Public Shadows Sub AcceptChanges()
  'this way AcceptChanges is 35 to 700 faster than Microsoft's way
  '35 is a speed factor when all rows were changed, and 700 is when a few
  'Tested on sets of 13K and 65K records
  Dim updatedRows() As DataRow = Me.Select(Nothing, Nothing, _
    DataViewRowState.Added Or DataViewRowState.Deleted Or _
    DataViewRowState.ModifiedCurrent Or DataViewRowState.ModifiedOriginal)
  For Each row As DataRow In updatedRows
    row.AcceptChanges()
  Next
End Sub

It appears to be from 35 to 700 times faster than Microsoft's implementation. Did I just uncover/solve a great mystery of ADO.NET, or am I missing something here?

share|improve this question
    
I am not fluent in VB, but there must be a reason why they put two try clauses in the function. My guess is that your function is more efficient but less robust. –  Dennis Jaheruddin Dec 28 '12 at 15:01
    
@DennisJaheruddin: there isn't anything VB-specific in the above code. Do you have trouble reading it? I can translate into C# for you, if you want. Wrapping it into a Try clause will not decrease performance. Main side of my question is when I will regret about changing to this implementation, instead of Microsoft's AcceptChanges. Every single test I did, performance wise, points that I am on the right track. –  Neolisk Dec 28 '12 at 15:51

1 Answer 1

It's quite easy to answer your question by looking at the DataTable.AcceptChanges source code.

As you can see they copy all DataRow objects into new array before iterating them. I suspect that your code will work incorrectly when rows are deleted (accepting changes for deleted rows means removing them from DataTable), most likely you'll get exception since it's not allowed to change collection while iterating it.

share|improve this answer
1  
Thanks for your feedback. Me.Select returns an array of DataRow, which is not the original DataRows collection. So I cannot see a problem here. –  Neolisk Dec 24 '12 at 21:45
    
A good question would be when SuspendIndexEvents and RestoreIndexEvents would play a part (i.e. make it faster than my implementation), if ever. –  Neolisk Dec 24 '12 at 21:46

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.