Link to home
Start Free TrialLog in
Avatar of GrahamLaws
GrahamLaws

asked on

Optimize recordset update - speed

How can the following code be optimized for speed:

Private Sub cmdBOM_Click()
Dim blnOK As Boolean
If Me.LMsuccess = "No" Then
MsgBox "Please Import LM data before processing", vbInformation, "Processing BOMs"
Else
MsgBox ("Files Found")
'in Tools | References, make sure the ADO '  "Microsoft ActiveX Data Objects 2.1 Library" is checked
Dim rst As ADODB.Recordset
Dim strSQL As String
Dim strPCode As String
Dim lngRecNo As Long
Dim lngRecCount As Long
Call acbInitMeter("BOM Manipulation", True)
lngRecCount = DCount("*", "tbom")
strSQL = "Select * From tbom"
Set rst = New ADODB.Recordset
rst.Open strSQL, CurrentProject.Connection, adOpenKeyset, _
      adLockOptimistic, adCmdText
If Not rst.BOF And Not rst.EOF Then
    'Initialize variables
    strPCode = rst.Fields("pCode").Value
    rst.MoveNext
Else
    MsgBox "There are NO records in the recordset!", _
      vbCritical, "No records Found!"
    rst.Close
    Set rst = Nothing
    Exit Sub
End If
lngRecNo = 0
Do While Not rst.EOF
 DoEvents
 blnOK = acbUpdateMeter((lngRecNo / lngRecCount) * 100)
    If Not blnOK Then
        With Me
        .BPsuccess = "No"
        .BPdate = ""
        End With
        Call acbCloseMeter
        rst.Close
        Set rst = Nothing
        'MsgBox "Operation Cancelled!", vbOKOnly + vbCritical, "Process BOMs"
        Exit Sub
    End If
    If rst.Fields("pCode").Value = "." Then
        rst.Fields("pCode").Value = strPCode
        rst.Update
    Else
        strPCode = rst.Fields("pCode").Value
    End If
    'Move to the next record
    lngRecNo = lngRecNo + 1
    rst.MoveNext
Loop

rst.Close
Set rst = Nothing
Call acbCloseMeter

DoCmd.SetWarnings False
DoCmd.RunSQL "SELECT pCode, Bqty, bUnit " & _
      "INTO tBuildUnit " & _
      "FROM tBom " & _
      "WHERE (((tBom.cCode) Is Null));"
DoCmd.SetWarnings True

MsgBox ("BOM processing completed successfully")

    With Me
        .BPsuccess = "Yes"
        .BPdate = Now()
    End With
End If
End Sub

Many thanks,

Graham
Avatar of Leigh Purvis
Leigh Purvis
Flag of United Kingdom of Great Britain and Northern Ireland image

So acbUpdateMeter just updates a progress meter?
Is that the reason why you're doing through a recordset - to be able to display progress to the user?
I'm not clear on why else you need a recordset - it looks like you're just updating every row where pCode = "." to the value from the first record.
(Which lends itself to an Update statement of course).

If you're determined to stay with the recordset then you should be able to remove the DoEvents - as that will be eating processor cycles that you likely don't need to.
(Will be noticable if there are many records in the table).
If tbom has many fields then you *might* get an improvement by restricting your selection to the fields that are actually involved e.g.

strSQL = "Select pCode From tbom"

instead of

strSQL = "Select * From tbom"

Avatar of GrahamLaws
GrahamLaws

ASKER

Hi Lpurvis,

Yes acbUpdateMeter is just a progress meter.
I'm using the recordset because I want it to roll through and
Replace pCode if it is "." with strPCode = rst.Fields("pCode").Value
i.e. If there is a code present it keeps and applies it to all the following records that have "." as pCode until it finds a pCode not equal to "." in which case that become the new string to be copied. E.g:

Table before process:
pCode     Field2     Field3
Code1     x            y
.              a            b
.              l             m            
Code2    d            e
.             f             g
.             a             s
etc

Table after process:
pCode     Field2     Field3
Code1     x            y
Code1    a            b
Code1    l             m            
Code2    d            e
Code2    f             g
Code2    a             s
etc

Regards

Graham
Hi Natchiket,
Gave it a try and doesn't seem to make a difference, It takes about 1 minute 20 seconds to run the process on 71k records. This is the same as when selecting '*'.
Thanks though.
Hi Lpurvis,

Meant to add i tried removing the 'Do Events' but it simply stops updating the progress meter and didn't increase the process time.
Is this an ActiveX Progress bar?  Or one you've rolled yourself?  (Which is usually as good and quite light).
The DoEvents removal is worthwhile if possible -  and if the Progress Bar were a native Access makedo one then a Repaint should be enough.

What Natchiket suggested is always worth doing (and you should keep doing it - even if you're not seeing a speed difference... You might often see experts here suggest SELECT * FROM in answers - that's because we usually don't know the field names involved ;-)

However - all that aside I see what you mean with your data example (thanks for that, it helps).
Does your table have a Primary Key (it *really* should) - perhaps an autonumber ID field?
Hi L,

Its basically just a simply form with a rectangle that repaints wider as the count approaches 100%. With a label saying x%:

Public Property Let Value(intValue As Integer)
   Me.recStatus.Width = CInt(Me.lblStatus.Width * (intValue / 100))
   Me.lblStatus.Caption = Format$(intValue, "##") & "% complete"
   DoCmd.RepaintObject
End Property

It gets the satus from:
acbUpdateMeter((lngRecNo / lngRecCount) * 100)
Forms(mconMeterForm).Value = intValue


When i took the Do Events out the progress form froze, and took the same time to complete?

Thank you both for the point about specific SELECT, I'll do that :)

Yes the tBom table has a PK that is an autonumber.

Would it help to index pCode?
or can i make my loop faster by moving some of the IF's outside it....?

Thanks Guys


SOLUTION
Avatar of Leigh Purvis
Leigh Purvis
Flag of United Kingdom of Great Britain and Northern Ireland image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Lpurvis / JimD,

Thanks for comments,

Ok with leaving Do Events in but taking out the check for cancel (blnOK) time dropped to 57s.
Taking Do Events out got this to 25s (Apologies LPurvis for my post at 11:10AM, my machine must have run something last time which made the meter freeze and not seem any quicker)
If I have no progress meter at all process time is 18s.

So i'm quite happy with 25s and a meter, at least the user knows whats going on.

Out of curiosity do recordsets write directly to the HDD? If so could memory variables be used and then written to the disk after processing? From the point of view that RAM is faster than disk?
Or should this be a new question / thread?

Thanks again,

Regards

GrahamL
Jim,

Yes good point, i probably should be using DAO, as i am elswhere in the DB.
1. Can you illustrate a 'mod' function, i have never used one before?
2. Yes 'do events' removed
3. Have restricted rst to 'pCode', is that what you mean by 'build SQL on the fly'?

Graham
Another question to add.... (should i be posting these as new questions?)

After running this process my DB inflates from 26MB on disk to 560MB, is there any 'clean up' / compact etc i can use? is there VB for this? e.g. DoCmd.Compact?!

Or just have compact on close selected in options?

GL
>>Or one you've rolled yourself? :o)

Quick Comment.

Does this work for you because you're NOT sorting the data in any particular way? It's FIFO? I can see where...if you don't have an index for these records you could quite likely create an entry that isn't correct.

I would recommend that before you ran any updates you include an index field just to keep the order of operation correct.

J
Hi Jeff,

Yes the output file i get from our system is in the form above, where effectively child records do not contain a linking field to their parent (pCode) so this loop is to update these records with the correct pCode, they always ouput sequentially e.g.

parent
child
child
child
parent
child
etc

the child lines / rows depend on how many components are needed to manufacture a given product.

the sort order is determined by BomID which is an autonumber field, and the data is imported fresh and not manipulated in any way prior to this recordset update.

Graham
ASKER CERTIFIED SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
sorry, I mis-typed the update SQL.

UPDATE tborn SET tborn.[pCode] = fillfield([pCode]);
            ^^^^

Jim,

I gave DAO a go using:

Private Sub cmdDAO_Click()
Dim lngRecCount As Long
Dim lngRecNo As Long
Dim strPCode As String
Dim MyDb As DAO.Database
Set MyDb = CurrentDb
Dim rsBom As DAO.Recordset
Dim strSQL As String
strSQL = "SELECT pCode FROM tBom"
Set rsBom = MyDb.OpenRecordset(strSQL)
Call acbInitMeter("Processing BOMs...", False)
lngRecCount = DCount("*", "tbom")
lngRecNo = 0
    With rsBom
        .MoveFirst
        Do While Not .EOF
            .Edit
            If rsBom.Fields("pCode").Value = "." Then
                rsBom.Fields("pCode").Value = strPCode
                .Update
            Else
                strPCode = rsBom.Fields("pCode").Value
            End If
        acbUpdateMeter ((lngRecNo / lngRecCount) * 100)
        lngRecNo = lngRecNo + 1
        rsBom.MoveNext
        Loop
    End With
rsBom.Close
Set rsBom = Nothing
Call acbCloseMeter

Which runs in about 22s, is this piece of code as good as it can be? (including a progress meter).

Jeff:

Will give your FillField method a try!

Graham
You mentioned earlier a noticable improvement without the progress meter.
So Jim's mentioning of reducing that by checking first with Mod is still worthwhile.

(Just out of interest - when you were still using ADO, did you try my recommendation of using a ForwardOnly cursor type?)
<<1. Can you illustrate a 'mod' function, i have never used one before?>>

  Usually calling a progress meter every time through a loop is just a waste of time.  In this case, your updating 71K records and since most meters only display based on whole percents, that would mean that you only really need to update the meter every 710 records.

  In addition, most users will think a program is slow if a meter is updated on every % (looks like it is just crawling along).  It's better to do it in jumps.  I usually shoot for 3 or 4 %.

  So you can use the mod function to reduce the number of times the meter is called.    Mod says take two numbers, divide, and hand me the remainder.  

  If lngRecNo Mod 1000 = 0 then
    blnOK = acbUpdateMeter((lngRecNo / lngRecCount) * 100)
  End If

  Above we are dividing by 1000.  Anything not evenly divisible 1000 will return a value.  So now your progress bar will only get called once every 1000 records (1000, 2000, 3000, 4000, etc) or 71 times rather then 71,000 times.  You might even want to jump that up to 1500 or 2000.

JimD
Lpurvis,

Yep, i did seemed to make a small improvement but only maybe a second.

Jim,

Will try your mod function. Thanks.
Jim,

That has shaved another 10 seconds off, making it now around 14s to run (i used 2500 in the end)

Thanks! Alot better than 1m 20s that i started with.

Graham




Jeff,

I don't understand where to use your code.... Sorry i'm pretty inexperienced with access.

if i have a cmd button it goes in there??

G
Hi G,
It's actually a simple query.

copy the SQL into a query SQL view and save it.

the query calls the function.

The function you can copy out of here and paste into a code module.

Create a backup of your table before you run the update...always.

Then just run the query.

Jeff,
Re. your function:

Function FillField(strin As String) As String
If strin <> "." Then
FillField = strin
Else
FillField = OldVal
End If
OldVal = FillField
End Function


I'm confused where does it get its value from if pCode <>"."  ?

i.e if it encounters "." it replaces it with 'fillfield'

tBom looks like:
pCode
Code1
.
.
Code2
.
.

After i run the update query all the "." are replaced with either null or maybe "".

Graham



Function FillField(strin As String) As String  <---strin is the value Code1 passed from the query
If strin <> "." Then                                  < first record is compared. Code1 <> . so
FillField = strin                                         <  the function returns Code1
Else
FillField = OldVal                                      <- If strIn = . then it means I don't have a new Code yet so use the old one
End If
OldVal = FillField                                      <--set the old value so I can use it in case of a .
End Function


Also, don't forget this

Dim OldVal As String  

it goes at the Top of your code module. It is a public variable used by the function.
another minor point ... don't change the mode of the rst to .Edit unless you really need to.

        Do While Not .EOF
            If rsBom.Fields("pCode").Value = "." Then
                .Edit
                rsBom.Fields("pCode").Value = strPCode
                .Update
            Else
                strPCode = rsBom.Fields("pCode").Value
            End If


hmmm ....
I also think you should use a variable in place of always forcing the CPU to allocate and populate "." an implied var each iteration through the loop

Dim strPeriod As String

strPeriod = "."

        Do While Not .EOF
            If rsBom.Fields("pCode").Value = strPeriod  Then
                .Edit
                rsBom.Fields("pCode").Value = strPCode
                .Update
            Else
                strPCode = rsBom.Fields("pCode").Value
            End If

all of these points gain more importance the more records you have to iterate.
if you really want to squeek the best performance from a recordset you can use the Hidden member .Collect ...

this does speed up the retrieval of a field value in a recordset ... the only trick is that you need to refer to thye field by it's ordinal position within the recordset ...

        Do While Not .EOF
            If rsBom.Collect(0) = strPeriod  Then
                .Edit
                rsBom.Fields("pCode").Value = strPCode
                .Update
            Else
                strPCode = rsBom.Collect(0)
            End If
Ah! sorry Jeff was putting the 'Dim OldVal As String' inside the function instead of in the public bit.
I have used:

Private Sub cmdBomSQL_Click()
DoCmd.SetWarnings False
DoCmd.RunSQL "UPDATE tBom SET tBom.pCode = fillfield([pcode]);"
DoCmd.SetWarnings True
End Sub

On the form and..........................it runs in about 2 seconds!

Also as a bonus you get the 'Run Query' progress indicator in the status bar negating the need for my own progress meter (with it only taking 2 secs i'm not to worried about indicating progress anyway, changing the caption to 'Processing' would do.

Now this was my 1st question on EE so i don't really know how to sort points / solutions out...

Jim's info was very helpful too for other things, though the SQL solution was fastest in the end. And I learned about 'Do Events' and where to place things in IFs etc. So everyone v helpful - thank you!!
hmmmm ... if the value of pCode is always either "." or a string that has more than 1 character you could speed it up a buit more by skipping the character comparison and use Len. Len shoukld be faster because it already has to stuff the value of the field into a variable and if you use a string variable it automatically stores the len in the last byte(or is it the first) and calling the Len function only has to read the value of the last byte as a number instead of loading strings all over again.

        Do While Not .EOF
            If Len(rsBom.Collect(0)) = 1  Then
                .Edit
                rsBom.Fields("pCode").Value = strPCode
                .Update
            Else
                strPCode = rsBom.Collect(0)
            End If
Graham,
I love recordsets too...but for speed straight SQL, even involving a small function call for each record has always proven faster. It's actually these guys (other experts) that have taught me that.

When speed doesn't matter...I too use recordsets when it's applicable because I enjoy coding.
Glad we got it going.
J
also Graham,

use

currentdb.execute "UPDATE tBom SET tBom.pCode = fillfield([pcode]);"

and you don't need the warning on and off parts.

J
Steve,

I'll give it a go out of the interest in learning :) but i'm pretty happy with the SQL update because it runs so quick.

Thanks though, smart point about len didn't think of that. (And had never heard of collect!!)



Graham

SQL is almost always faster, and yes I love to code, but some people are not comfortable with SQL ande thought youp were leaning to a "rst only" solution. If you really do play with the suggestions, please let us know if there was any perceptible difference, some of thios might only help if the recordset size becomes *really* large :-)

Steve
Thanks guys,

Jeff using the execute method seems instantaneous.... i'm amazed.

Originally after the pCode was replaced i then copied the 'Parent' lines to tBuildUnit with (i have replaced DoCmd.RunSQL with CurrentDb.Execute):

CurrentDb.Execute "DELETE * FROM tBuildUnit"
CurrentDb.Execute "SELECT pCode, Bqty, bUnit " & _
      "INTO tBuildUnit " & _
      "FROM tBom " & _
      "WHERE (((tBom.cCode) Is Null));"

But this now tells me, "Runtime 3010, table 'tBuildUnit' already exists".... Any thoughts?

Now having got this manipulation so fast i only wish i could get the csv import faster! Should that be a new thread or doesn't it matter?

I'm using (to get the data in):
DoCmd.TransferText acImportDelim, "tSales", "tSales", strMyDocuments & "\LM45010.CSV"
DoCmd.TransferText acImportDelim, "BomImport", "tBom", strMyDocuments & "\LM44211.CSV"
DoCmd.TransferText acImportDelim, "tUom", "tUom", strMyDocuments & "\LM44141.CSV"

45010 is 39k rows (6.38MB)
44211 is 71k rows (3.88MB)
44141 is 138k rows (3.93MB)

Thanks again. G
>>>CurrentDb.Execute "DELETE * FROM tBuildUnit"

this didn't delete the table, just the data

so you probably want to insert into vs Select into
CSV Import....new Q
Thanks Jeff, will do.

On the insert into part i now get "syntax error 3134, in INSERT INTO statement"
should i use DoCmd.RunSQL instead?
maybe something like so

"Insert Into tBuildUnit ( pCode, Bqty, bUnit ) select  SELECT pCode, Bqty, bUnit  from tBom WHERE ((tBom.cCode) Is Null);"

? you can always reconstruct this update query in your QBE builder. Just for the correct syntax.
Just to explain my points..... (think i should have assigned more than 125 in the end but i didn't know how to change it).

Lpurvis 20 because he helped first and pointed out 'Do Events' and moving my IFs around.
JimD 40 because he pointed both DAO, and the Mod function.
Jeff 65 because his solution was the fastest, and the question was speed.
I probably aught have given some to Stevebe as his suggestion sound sensible from a DAO pov but in honesty i didn't have the time to try it, and i couldn't see it beating the SQL method.

I hope that's fair, i'm new to this points business as its my 1st post. thank you all so much. If my points aren't fair or i aught to assign some more, please let me know!!
Graham ... I think you did a good job assigning points and also a better job of understanding and testing the things we all suggested. I can tell you from the view point of a frequent poster that we really appreciate someone who gets it, and typically that is more important to us than the exact point distribution ... well as long as it is in the ballpark :-)

Steve
Steve,

 << better job of understanding and testing the things we all suggested>>

  Well said.

  For me, I really don't care about the points.  It's more about solving the problem and possibly learning something in the process.

  But what always gets me is someone unwilling to try things and is simply looking to get the work done for them.

Graham,

<<I hope that's fair, i'm new to this points business as its my 1st post. thank you all so much. If my points aren't fair or i aught to assign some more, please let me know!!>>

  ya did great...hope you continue to find EE useful!

JimD
<<Chiming in>>
I too enjoyed the way you handled each suggestion in turn and compared your results. You seemed to learn in the process, and that makes it all worth it in my book. Experts = Teachers...to some. Experts = Quick Answers...for others. I MUCH prefer the first option.
H2H
Thanks also!
J