Function is killing performance

S

stephen.h.dow

Hi - I'm using a custom function in a query and it takes forever - was
hoping someone might have an idea of how to speed things up a bit.

A little more info: the function is relatively simple (4 arguments
that are run through an IF statement to pick a value from another
table (only 12 records). The query is run against a table with 41k
records. The numbers don't seem to big here, and the query takes over
an hour.

Any ideas??
 
P

Pieter Wijnen

Where & how are you using the IF's?
Sounds like a coding problem to me

Pieter
 
J

Jeff Boyce

Please post the SQL of your query and please post the code of your function.

Is the table underlying the query well-normalized? Is it properly indexed?

Is the table 1) within the same .mdb file as the query; 2) in a separte
..mdb file located on a LAN; 3) in a database/server other than Access
(e.g., SQL-Server, ...)?

Regards

Jeff Boyce
Microsoft Office/Access MVP
 
S

stephen.h.dow

looks like this:

If Arg1=a and Arg2=b then
with (small table)
if Arg1=field1 and Arg2=field2 and Arg3=field3 and
Arg4=field4 then func_result = 1
.movenext
endwith
elseif Arg1=c and Arg2=d then
....
elseif Arg1=e and Arg2=f then
....
endif

That's pretty much it...
 
D

Dirk Goldgar

In
Hi - I'm using a custom function in a query and it takes forever - was
hoping someone might have an idea of how to speed things up a bit.

A little more info: the function is relatively simple (4 arguments
that are run through an IF statement to pick a value from another
table (only 12 records). The query is run against a table with 41k
records. The numbers don't seem to big here, and the query takes over
an hour.

Any ideas??
Click to expand...

You should probably post the code of the function. You mention that the
function "pick a value from another table". Are you using DLookup to
do that? Or opening a recordset each time the function is called?
Either way, that's going to drag you down horribly. Maybe you can
rewrite the whole query to join the lookup table so as to avoid having
to do that. Or else, load the values from that table into static
variables the first time the function is called, so you don't have to
look them up again.
 
S

stephen.h.dow

To answer Jeff's questions:
Everything is in the same mdb file

the sql is:
SELECT whse_id, newsegment(crdt_sc,coapp_crdt_sc,cosign,et,whse_id) AS
RevisedSegment INTO testtable
FROM workingdata

the newsegment function is:
Public Function NewSegment(AppFico As Variant, CosFico As Variant,
CosiFlag As String, ET As Variant, Optional RecID As Variant) As
Integer
' RecId is just for error-checking

Dim qd As TableDef
Dim rs As Recordset
Dim db As Database
Dim TierMatch As Integer

Set db = CurrentDb
Set qd = db.TableDefs("NewBands")
Set rs = qd.OpenRecordset()
TierMatch = 0

If CosiFlag = "Y" And IsNull(AppFico) Then
With rs
.MoveFirst
Do Until TierMatch = 1
If (.Fields("Cosign") = "Y"
And .Fields("AppFICO_NoScore_Okay") = "Y" And _
CosFico >= .Fields("mincosfico") And ET
= .Fields("ExpTier")) Then
NewSegment = .Fields("tier")
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
End With
ElseIf CosiFlag = "Y" And AppFico > 0 Then
With rs
.MoveFirst
Do Until TierMatch = 1
If (.Fields("Cosign") = "Y" And CosFico
= .Fields("mincosfico") And _
Click to expand...
AppFico >= .Fields("minappfico") And ET
= .Fields("ExpTier")) Then
NewSegment = .Fields("tier")
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
End With
ElseIf CosiFlag = "N" Then
With rs
.MoveFirst
Do Until TierMatch = 1
If .EOF = True Then GoTo Assigned
If (.Fields("Cosign") = "N" And AppFico
= .Fields("minappfico")) Then
Click to expand...
NewSegment = .Fields("tier")
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
End With
Else: NewSegment = 100
End If

Assigned:

End Function
 
P

Pieter Wijnen

try to minimize the db calls (looping through 12 times per record), Even
DLookup beats that <g>

If Arg1=a and Arg2=b Then
Db.OpenRecordset("SELECT 'X' FROM TableA WHERE Field1=" & Arg1 & " AND
Field2=" & Arg2 ....",DAO.dbOpenSnapshot)
MyFunc = Abs(Not Rs.EOF)
Rs.Close : Set Rs = Nothing
Exit Function
Else ...

HTH

Pieter
 
A

Albert D. Kallal

To open a query, time setup is rather large. You can likely process 10,000,
or even 100,000 records in the time it takes to open ONE query, and set a
reocrset.

Which is faster, walking across a street, or using a helicopter to cross the
street?

Well, by the time you go through the flight check list, turn on the fuel
pumps, and get the blades up to speed, you likely could have crossed the
street 10 times by walking.

However, once that helicopter is up to speed, then the time to fly across
the street is barely noticeable (assuming the copter is up to full speed
WHEN it starts to cross the street).

Anytime you open a table for processing, you are kind
starting up a helicopter. It takes an enormous amount of time to do this.
(the sql has to be parsed, the sql engine has to load, has initialized,
build a query plan, and THEN start to grab the data).

Once the helicopter (or recordset) is open, then you can grab, and process
data very fast...

You can read thousands of records per second with ease, but you can only
execute maybe 10, perhaps 25 queries per second. So, keep this concept in
mind when trying to optimize
your code...

In place of using some queries and parameters, you could open a
reocrdset, and KEEP it open. Then use find firsts....and that would elooate
the expensive and huge helicopter start-up time of opening the table is done
ONCE.

You can't affored to open up a table for each row of the query...it too
slow...

However, if you can use a join, or a sub-query for each row, then that will
be fast. You could use a sub-query, and that does allow you to use "seval"
values from each row...

select FirstName, LastName, co, p1, p2
(select myValue from tbL where tblL.parm1 = tblMain.p1 and tblL.parm2 =
main.p2) as myresults)

from tblMain


A sub-query will be fast, and you can use a select to pull values based on
several params as shown).
 
S

stephen.h.dow

I like the analogy, Albert. Unfortuantely, a query won't work (I don't
think) based on the logical tests I have in the fucntion. I'll keep
plugging away...
 
D

Dirk Goldgar

In
To answer Jeff's questions:
Everything is in the same mdb file

the sql is:
SELECT whse_id, newsegment(crdt_sc,coapp_crdt_sc,cosign,et,whse_id) AS
RevisedSegment INTO testtable
FROM workingdata

the newsegment function is:
Public Function NewSegment(AppFico As Variant, CosFico As Variant,
CosiFlag As String, ET As Variant, Optional RecID As Variant) As
Integer
' RecId is just for error-checking

Dim qd As TableDef
Dim rs As Recordset
Dim db As Database
Dim TierMatch As Integer

Set db = CurrentDb
Set qd = db.TableDefs("NewBands")
Set rs = qd.OpenRecordset()
TierMatch = 0

If CosiFlag = "Y" And IsNull(AppFico) Then
With rs
.MoveFirst
Do Until TierMatch = 1
If (.Fields("Cosign") = "Y"
And .Fields("AppFICO_NoScore_Okay") = "Y" And _
CosFico >= .Fields("mincosfico") And ET
= .Fields("ExpTier")) Then
NewSegment = .Fields("tier")
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
End With
ElseIf CosiFlag = "Y" And AppFico > 0 Then
With rs
.MoveFirst
Do Until TierMatch = 1
If (.Fields("Cosign") = "Y" And CosFico
AppFico >= .Fields("minappfico") And ET
= .Fields("ExpTier")) Then
NewSegment = .Fields("tier")
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
End With
ElseIf CosiFlag = "N" Then
With rs
.MoveFirst
Do Until TierMatch = 1
If .EOF = True Then GoTo Assigned
If (.Fields("Cosign") = "N" And AppFico
NewSegment = .Fields("tier")
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
End With
Else: NewSegment = 100
End If

Assigned:

End Function
Click to expand...

Yes, that function is going to kill performance. If the table
"NewBands" is small, you might consider loading a simple, static array
from the table, the first time the function is called, and then have
every call to the function loop through the array rather than opening,
looping through, and closing a recordset on the table.

It may also be that you can set up your query very cleverly so that you
don't have to call a function at all; however, the logic for that would
be more complicated than I have time now to work out.
 
S

stephen.h.dow

In











Yes, that function is going to kill performance. If the table
"NewBands" is small, you might consider loading a simple, static array
from the table, the first time the function is called, and then have
every call to the function loop through the array rather than opening,
looping through, and closing a recordset on the table.

It may also be that you can set up your query very cleverly so that you
don't have to call a function at all; however, the logic for that would
be more complicated than I have time now to work out.

--
Dirk Goldgar, MS Access MVPwww.datagnostics.com

(please reply to the newsgroup)- Hide quoted text -

- Show quoted text -
Click to expand...

Thanks, Dirk. Do you (or anyone else) have an example of the static
array - I am unfamiliar with that technique. Thanks again.
 
P

Pieter Wijnen

Simple Function To Load Static Data & Reuse afterwards

Public Function Foo(.............) As Long
Const MaxRows =100
Static Entries As Variant
Static hasRun As Boolean
Dim Rs As DAO:RecordSet
Dim i As Long


If hasRun = False Then
Set Rs = CurrentDb.OpenRecordset("SELECT ..... ", DAO.DbOpenSnapshot)
Entries = Rs.GetRows(MaxRows)
HasRun = True
End if

For i = LBound(Entries) To UBound(Entries)
If Entries(i)(0) = "A" And Entries(i)(0)="B" Then
Foo = 1
Exit For
ElseIf ....
End If
Next 'i
End Function

HTH

Pieter
 
A

Albert D. Kallal

*...use 'static' declares...

An *excellent* suggestion here by Pieter.

I would try this, as it means you don't have to re-design your code....

Try this suggestion..and post back as to the performance increase.....


You could even optimizes further, but then your existing code would have to
be re-written...
 
A

Albert D. Kallal

Thanks, Dirk. Do you (or anyone else) have an example of the static
array - I am unfamiliar with that technique. Thanks again.
Click to expand...

Before you go that re-coding way, try using a global, or "static" declare of
the reocrdset, so you only open it once.

Pieter has posted some code....

Pieter example is good because you don't have to re-write your existing
code.

if that performance increase is good enough, then you avoid having to
re-write code...

You could also use "seek" command for addtional speed increase (but, some
additonal code is needed for split databases).
 
P

Pieter Wijnen

Thx Albert

I Don't like the code from a puristic "what if the lookup table is updated
during the session" stance, but some tables do luckily stay static <g>

Pieter
 
D

Dirk Goldgar

In
Albert D. Kallal said:
Before you go that re-coding way, try using a global, or "static"
declare of the reocrdset, so you only open it once.
Click to expand...

I don't know if that's such a great idea, as I think it will require
additional code to run at some other point, after the completion of the
query, to close the recordset. Otherwise one runs the risk of having
Access not close due to the dangling object reference.

I also suspect that looping through an array is going to be faster than
looping through a recordset.
Pieter has posted some code....

Pieter example is good because you don't have to re-write your
existing code.
Click to expand...

Did Pieter post code for a static recordset? I don't see it. He gave a
nice example using a static array.
 
A

Albert D. Kallal

Did Pieter post code for a static recordset? I don't see it. He gave a
nice example using a static array
Click to expand...


Thanks...I thought he just declared a Boolean, and then use the *same*
code...I see he actually did put in a array.... (so, I read what I assumed
he done!!!).

I still like the static declare of the recordset, and a Boolean to test for
this.

I could make the case that I don't like the existing code, and it should be
re-written. However, that not really the best kind of help!! I am a big fan
of solutions that allow the minimal amount of work, and allows the users to
keep much what they done.

Perhaps I been around Larry, Tony and you too much, and I learned to be nice
to everyone now!. And, I like to suggest the most easy way possible!!!

If I were the poster, I would try a static declare, and Boolean for the
recordset open code...and leave the rest much the same as it is now.

Case in point noted....I have litte doubt the array would be faster.......
 
G

Guest

Large optimisation starts at the Outside.

You should be using SQL, not calling a function.
You should be using a function, not calling SQL
You should be using a static tabledef object.
You should be using a select query (for ODBC) or
You should be using an index (for a TableDef object)
You should be using field variables inside the loop.

The last one is the easiest change, and even that makes
an enormous difference -- although it will still be orders
of magnitude slower than optimisation starting with
changing the method.

--------------
dim fld_cosign as dao.field
dim fld_OK as dao.field
dim fld_fico as dao.field
dim fld_exp as dao.field
dim fld_tier as dao.field

set fld_cosign = .Fields("Cosign")
set fld_ok = .Fields("AppFICO_NoScore_Okay")
set fld_fico = .Fields("mincosfico")
set fld_exp = Fields("ExpTier")
set fld_tier = .Fields("tier")

Do Until TierMatch = 1
If (fld_cosign = "Y" And fld_OK = "Y" And _
CosFico >= fld_fico And ET = .fld_exp) Then
NewSegment = fld_tier
TierMatch = 1
GoTo Assigned
End If
.MoveNext
Loop
 
Top