Not optimal code

domel · Jan 14, 2009

Hi.
I use temp-tables in my code and its so slow that it takes ours to create a raport.. I don't know what i am doing wrong with temp-tables.. Please help me if You can.
Here it is:

DEFINE VARIABLE data_u LIKE wo_rel_date NO-UNDO INITIAL TODAY.
DEFINE VARIABLE data_u1 LIKE wo_rel_date NO-UNDO INITIAL TODAY.
DEFINE VARIABLE buyer1 LIKE wo_cc.
DEFINE VARIABLE buyer2 LIKE wo_cc.
DEFINE VARIABLE buyer3 LIKE wo_cc.
DEFINE VARIABLE part1 LIKE wo_part.
DEFINE VARIABLE part2 LIKE wo_part.
DEFINE VARIABLE pierwszy AS LOGICAL INITIAL TRUE.
DEFINE VARIABLE lp AS INTEGER FORMAT 99999 INITIAL 0.
DEFINE VARIABLE lp2 AS INTEGER FORMAT 99999 INITIAL 0.
DEFINE VARIABLE zapas LIKE ld_qty_oh FORMAT "->>>>>9.99".
DEFINE VARIABLE Zapas_mag LIKE ld_qty_oh FORMAT "->>>>>9.99".
DEFINE VARIABLE ilosc_tym LIKE wod_qty_req FORMAT "->>>9.99".
DEFINE VARIABLE tym_id LIKE wo_lot.

DEFINE TEMP-TABLE ZR
FIELD lp AS INTEGER FORMAT 99999
FIELD woreldate LIKE wo_rel_date
FIELD id LIKE wo_lot
FIELD indeks_z LIKE wo_part
FIELD indeks_k LIKE wod_part
FIELD ilosc LIKE wod_qty_req FORMAT "->>>9.99"
FIELD zapas_p LIKE ld_qty_oh FORMAT "->>>>>9.99"
FIELD zapas_k LIKE ld_qty_oh FORMAT "->>>>>9.99"
FIELD osodp LIKE wo_cc
FIELD opis LIKE pt_desc2
FIELD zszp LIKE wo_so_job
FIELD grupa LIKE pt_group
FIELD nazwa LIKE pt_desc1
FIELD proj LIKE wo_project FORMAT "x(3)"
INDEX lp lp
INDEX sort1 osodp indeks_k woreldate ID zapas_p.

DEFINE BUFFER b_ZR FOR ZR.
bcdparm = "".

FOR EACH wo_mstr NO-LOCK WHERE
wo_due_date >= data_u AND wo_rel_date >= data_u AND wo_rel_DATE <= data_u1
AND wo_cc >= buyer1 AND wo_cc <= buyer2
AND wo_part >= part1 AND wo_part <= part2
AND (wo_status = "R")
GROUP BY wo_cc:

FOR EACH wod_det WHERE wod_lot = wo_lot:
FIND pt_mstr WHERE pt_part = wod_part NO-LOCK NO-ERROR.

ilosc_tym = wod_qty_req - wod_qty_iss.

IF ilosc_tym > 0 THEN DO:

IF pt_pm_code <> "P" THEN DO:

lp = lp + 1.
CREATE ZR.
ZR.lp = lp.
ZR.woreldate = wo_rel_date.
ZR.id = wo_lot.
ZR.indeks_z = wo_part.
ZR.indeks_k = wod_part.
ZR.ilosc = wod_qty_req - wod_qty_iss.
ZR.zapas_p = 0.
ZR.zapas_k = 0.
ZR.osodp = pt_buyer.
ZR.opis = pt_desc2.
ZR.zszp = wo_so_job.
ZR.grupa = pt_group.
ZR.nazwa = pt_desc1.
ZR.proj = SUBSTR(wo_project,1,3).
END. /*IF 0/1*/

END. /*IF*/
END. /*wod_det*/

END. /*wo_mstr*/

FOR EACH ZR USE-INDEX sort1 GROUP BY indeks_k BY woreldate BY id :

IF FIRST-OF (indeks_k) THEN DO:
zapas_mag = 0.
FOR EACH ld_det WHERE ld_part = ZR.indeks_k :
zapas_mag = zapas_mag + ld_qty_oh.
END.
END.

zapas_k = zapas_mag.
zapas_p = zapas_mag - ilosc.
zapas_mag = zapas_p.

END. /*ZR*/

FOR EACH ZR USE-INDEX sort1 GROUP by ZR.osodp BY ZR.indeks_k BY ZR.woreldate BY ZR.ID BY ZR.zapas_p DESC:

FIND FIRST pt_mstr WHERE pt_part = ZR.indeks_z NO-LOCK NO-ERROR.

FIND FIRST b_ZR WHERE b_ZR.indeks_k = ZR.indeks_k AND zapas_p < 0 NO-LOCK NO-ERROR.
IF AVAILABLE b_ZR THEN DO:
IF FIRST-OF (ZR.indeks_k) THEN lp2 = lp2 + 1.

PUT
lp2 AT 1
ZR.indeks_k AT 7
ZR.osodp FORMAT "X(4)" AT 26
ZR.grupa AT 31
ZR.nazwa AT 40
ZR.opis AT 65
ZR.woreldate AT 90
ZR.id AT 99
ZR.proj AT 108
ZR.zszp AT 114
pt_buyer FORMAT "X(4)" AT 123
ZR.indeks_z AT 128
ZR.ilosc AT 191
ZR.zapas_k AT 200
ZR.zapas_p AT 211
SKIP.
END.
DELETE ZR.
END.

Please help!

sdjensen · Jan 14, 2009

Without knowing what wo_mstr look likes it is hard to tell where the problem is.

But I would create an extra index for zr with index_k to speed up the following statement.

FIND FIRST b_ZR WHERE b_ZR.indeks_k = ZR.indeks_k AND zapas_p < 0 NO-LOCK NO-ERROR.

The zapas_p should be written as b_ZR.zapas_p or ZR.zapas and included in the index as well.

Casper · Jan 14, 2009

Well for starters look at the xref output and see what indexes are used.

second rethink your query so that only results which should create a temp-table are present. Right now I see two nested for eaches which IMO should be 1 for each, you can also incorporate the find in the for each and leave out the group-by. The group-by isn't necessary because you only fill a temp-table which can be sorted later on.
Put the pt_pm_code <> 'P' also in the for each.

If you have done that, do an xref again and see of the right indexes are used. If so retry the report.

Casper.

(BTW: I know nothing of mfg_pro (which this seems to be) and I assume this is all the code there is)

domel · Jan 14, 2009

Thanks! I did what You both said.
Here is my new code:
It is faster (almost 30%!! - thanks!!) but not fast enough.. I think it is a problem with temp-tables.. i have to use it 3 times and they are slow somehow..
If You have any other ideas please help!
What is it xref output? i don't know how to create it.. here is my new code:

DEFINE VARIABLE data_u LIKE wo_rel_date NO-UNDO INITIAL TODAY.
DEFINE VARIABLE data_u1 LIKE wo_rel_date NO-UNDO INITIAL TODAY.
DEFINE VARIABLE buyer1 LIKE wo_cc.
DEFINE VARIABLE buyer2 LIKE wo_cc.
DEFINE VARIABLE buyer3 LIKE wo_cc.
DEFINE VARIABLE part1 LIKE wo_part.
DEFINE VARIABLE part2 LIKE wo_part.
DEFINE VARIABLE pierwszy AS LOGICAL INITIAL TRUE.
DEFINE VARIABLE lp AS INTEGER FORMAT 99999 INITIAL 0.
DEFINE VARIABLE lp2 AS INTEGER FORMAT 99999 INITIAL 0.
DEFINE VARIABLE zapas LIKE ld_qty_oh FORMAT "->>>>>9.99".
DEFINE VARIABLE Zapas_mag LIKE ld_qty_oh FORMAT "->>>>>9.99".
DEFINE VARIABLE ilosc_tym LIKE wod_qty_req FORMAT "->>>9.99".
DEFINE VARIABLE tym_id LIKE wo_lot.

DEFINE TEMP-TABLE ZR
FIELD lp AS INTEGER FORMAT 99999
FIELD woreldate LIKE wo_rel_date
FIELD id LIKE wo_lot
FIELD indeks_z LIKE wo_part
FIELD indeks_k LIKE wod_part
FIELD ilosc LIKE wod_qty_req FORMAT "->>>9.99"
FIELD zapas_p LIKE ld_qty_oh FORMAT "->>>>>9.99"
FIELD zapas_k LIKE ld_qty_oh FORMAT "->>>>>9.99"
FIELD osodp LIKE wo_cc
FIELD opis LIKE pt_desc2
FIELD zszp LIKE wo_so_job
FIELD grupa LIKE pt_group
FIELD nazwa LIKE pt_desc1
FIELD proj LIKE wo_project FORMAT "x(3)"
INDEX sort1 AS PRIMARY osodp indeks_k woreldate ID zapas_p DESC
INDEX szukaj indeks_k zapas_p.
DEFINE BUFFER b_ZR FOR ZR.
bcdparm = "".

FOR EACH wo_mstr NO-LOCK WHERE
wo_due_date >= data_u AND wo_rel_date >= data_u AND wo_rel_DATE <= data_u1

AND wo_cc >= buyer1 AND wo_cc <= buyer2
AND wo_part >= part1 AND wo_part <= part2
AND (wo_status = "R"),
EACH wod_det WHERE wod_lot = wo_lot NO-LOCK,
EACH pt_mstr WHERE pt_part = wod_part AND pt_pm_code <> "P":

ilosc_tym = wod_qty_req - wod_qty_iss.

IF ilosc_tym > 0 THEN DO:

lp = lp + 1.
CREATE ZR.
ZR.lp = lp.
ZR.woreldate = wo_rel_date.
ZR.id = wo_lot.
ZR.indeks_z = wo_part.
ZR.indeks_k = wod_part.
ZR.ilosc = wod_qty_req - wod_qty_iss.
ZR.zapas_p = 0.
ZR.zapas_k = 0.
ZR.osodp = pt_buyer.
ZR.opis = pt_desc2.
ZR.zszp = wo_so_job.
ZR.grupa = pt_group.
ZR.nazwa = pt_desc1.
ZR.proj = SUBSTR(wo_project,1,3).

END. /*IF*/

END. /*wo_mstr*/

FOR EACH ZR USE-INDEX sort1 GROUP BY indeks_k BY woreldate BY id :

IF FIRST-OF (indeks_k) THEN DO:
zapas_mag = 0.
FOR EACH ld_det WHERE ld_part = ZR.indeks_k :
zapas_mag = zapas_mag + ld_qty_oh.
END.
END.

zapas_k = zapas_mag.
zapas_p = zapas_mag - ilosc.
zapas_mag = zapas_p.

END. /*ZR*/

FOR EACH ZR USE-INDEX sort1 GROUP by ZR.osodp BY ZR.indeks_k BY ZR.woreldate BY ZR.ID BY ZR.zapas_p DESC:

FIND FIRST pt_mstr WHERE pt_part = ZR.indeks_z NO-LOCK NO-ERROR.

FIND FIRST b_ZR WHERE b_ZR.indeks_k = ZR.indeks_k AND b_ZR.zapas_p < 0 USE-INDEX szukaj NO-LOCK NO-ERROR.
IF AVAILABLE b_ZR THEN DO:
IF FIRST-OF (ZR.indeks_k) THEN lp2 = lp2 + 1.

PUT
lp2 AT 1
ZR.indeks_k AT 7
ZR.osodp FORMAT "X(4)" AT 26
ZR.grupa AT 31
ZR.nazwa AT 40
ZR.opis AT 65
ZR.woreldate AT 90
ZR.id AT 99
ZR.proj AT 108
ZR.zszp AT 114
pt_buyer FORMAT "X(4)" AT 123
ZR.indeks_z AT 128
ZR.ilosc AT 191
ZR.zapas_k AT 200
ZR.zapas_p AT 211
SKIP.
END.
DELETE ZR.
END.

sdjensen · Jan 14, 2009

Why are you deleting the ZR record after printing?

You can "empty temp-table ZR" after the loop instead.

compile <programname> xref <xref-output-file>

Search xref.output-file for full index scans for faster processing.

Alternative you can use the function etime to see where you code is spending its time.
etime(true).
<code>
display etime.

This will show how many milliseconds the code took.

domel · Jan 14, 2009

Thx again.
I used etime in different areas and it look like the most time (220 out of 311) i spend in this fragment of the code:

FOR EACH wo_mstr NO-LOCK WHERE
wo_due_date >= data_u AND wo_rel_date >= data_u AND wo_rel_DATE <= data_u1

AND wo_cc >= buyer1 AND wo_cc <= buyer2
AND wo_part >= part1 AND wo_part <= part2
AND (wo_status = "R"),
EACH wod_det WHERE wod_lot = wo_lot NO-LOCK,
EACH pt_mstr WHERE pt_part = wod_part AND pt_pm_code <> "P":

ilosc_tym = wod_qty_req - wod_qty_iss.

IF ilosc_tym > 0 THEN DO:

lp = lp + 1.
CREATE ZR.
ZR.lp = lp.
ZR.woreldate = wo_rel_date.
ZR.id = wo_lot.
ZR.indeks_z = wo_part.
ZR.indeks_k = wod_part.
ZR.ilosc = wod_qty_req - wod_qty_iss.
ZR.zapas_p = 0.
ZR.zapas_k = 0.
ZR.osodp = pt_buyer.
ZR.opis = pt_desc2.
ZR.zszp = wo_so_job.
ZR.grupa = pt_group.
ZR.nazwa = pt_desc1.
ZR.proj = SUBSTR(wo_project,1,3).

END. /*IF*/

END. /*wo_mstr*/

So i have been wrong thinking it is problem of reading temp-table.. So is it a problem with filling it?

Any more helpful ideas maybe?

Casper · Jan 14, 2009

with xref you see which indexes are used:

Code:

compile <prograname> xref <locationxrefoutput/xref.output>

look for <whole index> for starters.
l
This information together with the indexes and fields of those indexes gives information on what the problem is.

How many records are we talking about here?

Casper.

parul · Jan 14, 2009

I don't know if it will be an overkill in this case but in general profiler is also a good way of seeing which line of the code is taking most time.

have a look at KB id 19495 for more about profiler system handle.

-Parul.

domel · Jan 14, 2009

i don't have this xref.. On my hole comupter there is no file which is called xref.. i cannot use it..
Records.. Hmm.. Around 40 000 baybe more.. in the temp-table.

i can put indexes thich are on these tables:

Table: pt_mstr

Flags Index Name St Area Cnt Field Name
----- ----------------------------- ------- --- ------------------------------
pt_bom_code N/A 2
+ pt_bom_code
+ pt_part

u pt_desc N/A 3
+ pt_desc1
+ pt_desc2
+ pt_part

u pt_group N/A 2
+ pt_group
+ pt_part

u pt_ll_mrp_pt N/A 3
+ pt_ll_code
+ pt_mrp
+ pt_part

pt_model N/A 1
+ pt_model

pt_network N/A 2 + pt_network
+ pt_part

pu pt_part N/A 1
+ pt_part

u pt_part_type N/A 2
+ pt_part_type
+ pt_part

pt_pm_mrp N/A 2
+ pt_pm_mrp
+ pt_part

u pt_prod_part N/A 2
+ pt_prod_line
+ pt_part

pt_pvm_bom N/A 2
+ pt_pvm_bom
+ pt_part

u pt_routing N/A 2
+ pt_routing
+ pt_part

Table: wo_mstr

Flags Index Name St Area Cnt Field Name
----- ----------------------------- ------- --- ------------------------------
wo_due_part N/A 3 + wo_due_date
+ wo_part
+ wo_site

wo_fsm_type N/A 3 + wo_fsm_type
+ wo_nbr
+ wo_itm_line

u wo_lot N/A 1 + wo_lot

pu wo_nbr N/A 2 + wo_nbr
+ wo_lot

wo_part N/A 3 + wo_part
+ wo_site
+ wo_due_date

wo_part_rel N/A 3 + wo_part
+ wo_site
+ wo_rel_date

u wo_type_nbr N/A 3 + wo_type
+ wo_nbr
+ wo_lot

wo_type_part N/A 4 + wo_type
+ wo_part
+ wo_site
+ wo_due_date

Table: wod_det

Flags Index Name St Area Cnt Field Name
----- ----------------------------- ------- --- ------------------------------
pu wod_det N/A 3 + wod_lot
+ wod_part
+ wod_op

wod_iss_part N/A 2 + wod_iss_date
+ wod_part

wod_nbrpart N/A 3 + wod_nbr
+ wod_lot
+ wod_part

u wod_op N/A 3 + wod_lot
+ wod_op
+ wod_part

wod_part N/A 1 + wod_part

i have the code like this right now:

FOR EACH wo_mstr NO-LOCK WHERE
wo_due_date >= data_u AND wo_rel_date >= data_u AND wo_rel_DATE <= data_u1

AND wo_cc >= buyer1 AND wo_cc <= buyer2
AND wo_part >= part1 AND wo_part <= part2
AND (wo_status = "R"),
EACH wod_det WHERE wod_lot = wo_lot USE-INDEX wod_det NO-LOCK,
EACH pt_mstr WHERE pt_part = wod_part AND pt_pm_code <> "P" USE-INDEX pt_part:

ilosc_tym = wod_qty_req - wod_qty_iss.

IF ilosc_tym > 0 THEN DO:

lp = lp + 1.
CREATE ZR.
ZR.lp = lp.
ZR.woreldate = wo_rel_date.
ZR.id = wo_lot.
ZR.indeks_z = wo_part.
ZR.indeks_k = wod_part.
ZR.ilosc = wod_qty_req - wod_qty_iss.
ZR.zapas_p = 0.
ZR.zapas_k = 0.
ZR.osodp = pt_buyer.
ZR.opis = pt_desc2.
ZR.zszp = wo_so_job.
ZR.grupa = pt_group.
ZR.nazwa = pt_desc1.
ZR.proj = SUBSTR(wo_project,1,3).

END. /*IF*/

END. /*wo_mstr*/

please help if You can.. thx anyway.

TomBascom · Jan 14, 2009

domel said:
Thx again.
I used etime in different areas and it look like the most time (220 out of 311) i spend in this fragment of the code:

If you're going to show code samples you ought to embed them in CODE tags. It makes it a lot more readable.

How long does this take to run?

Code:

define variable lr as integer no-undo.

find _myconnection no-lock.

find _userio no-lock where
  _userio._userio-usr = _myconnection._myconn-userid no-error.

lr = _userio._userio-dbaccess.
etime( yes ).
FOR EACH wo_mstr NO-LOCK WHERE 
    wo_due_date >= data_u  AND wo_rel_date >= data_u AND wo_rel_DATE <= data_u1 

                           AND   wo_cc >= buyer1 AND wo_cc <= buyer2
                            AND wo_part >= part1 AND wo_part <= part2
                            AND (wo_status = "R"),
                            EACH wod_det WHERE wod_lot = wo_lot NO-LOCK,
                            EACH pt_mstr WHERE pt_part = wod_part AND pt_pm_code <> "P":
END.
display etime.

Code:

  ilosc_tym = wod_qty_req - wod_qty_iss.

  IF  ilosc_tym > 0 THEN DO:
    lp = lp + 1.
    CREATE ZR.

In the block above you are throwing away some records that you fetched, if this is a very small percentage of the result set that's ok but if it is most of the results set it is a very inefficient way to code. Ideally you would have had something like:

Code:

      AND wod_qty_req > wod_qty_iss

as part of the WHERE clause.

This next bit is a small thing and unlikely to be your problem but when assigning multiple values do it like this:

Code:

    assign
      ZR.lp = lp
      ZR.woreldate = wo_rel_date
      ZR.id = wo_lot
      ZR.indeks_z = wo_part
      ZR.indeks_k = wod_part
      ZR.ilosc = wod_qty_req - wod_qty_iss
      ZR.zapas_p = 0
      ZR.zapas_k = 0
      ZR.osodp = pt_buyer
      ZR.opis = pt_desc2
      ZR.zszp = wo_so_job
      ZR.grupa = pt_group
      ZR.nazwa = pt_desc1
      ZR.proj = SUBSTR(wo_project,1,3)
    .

(Note that there is no "." after each line...)

Code:

  end.
end.

find _userio no-lock where
  _userio._userio-usr = _myconnection._myconn-userid no-error.
lr = _userio._userio-dbaccess - lr.

display lr lp ( lr / lp ).

This last bit will tell us how many "logical reads" you program executed vs how many records you actually did something with. A "logical read" (or 'db access") occurs when Progress access a block in the database. Every record read requires at least 2 logical reads -- one for the index block and one for the data block. Thus the expected ratio between "useful records" (as counted by your "lp" variable) and logical reads should be roughly 2 for an efficient program processing a single table. In your case you are joining 3 tables so I would expect it to be around 6 if your code is using appropriate indexes. My guess is that the actual value is much, much higher than 6.

[/quote]So i have been wrong thinking it is problem of reading temp-table.. So is it a problem with filling it?

Any more helpful ideas maybe?[/QUOTE]

Casper has been trying to tell you that it is much more likely that the issue is the amount of time that it takes your query to run.

This might be because it lacks proper indexes to support the WHERE clause. Which is why the XREF output is important. If the XREF shows WHOLE-INDEX searches that means that you are likely reading the entire table to satisfy the query rather than only reading the records that you actually need. For instance, if the table contains 1,000,000,000 records and your report only needs 1,000 of them you would be doing 1,000,000 times as much work as needed to prepare the report. This is, by far, the most common source of application performance problems and is the number one area that you, as a programmer, need to understand in order to write efficient code.

domel · Jan 14, 2009

Ok. i changed the code and put yours fragments in it.
the report was for all the data (the longest).
etime = 323,01
etime - hole program - 551

summaring
lr lp
--- ------
0 61838 0.00

not all 10 minutes is not as bad already.

If You have any more suggestions please reply.

TomBascom · Jan 14, 2009

Sorry, but I cannot understand your reply.

TomBascom · Jan 14, 2009

However... lp = 61838 does say that you are writing a lot of temp-table records. So you would want to make doubly sure that the temp-table related client startup parameters are adequate. What version of Progress is this? What OS?

Potentially relevant startup parameters include:

-T # specifies where to put temporary files
# use a disk with lots of spare IO operations capacity
-t # Make the -T files visible so that you can see how big they are
# temp-tables go to files whose name starts with "DBI"
-Bt # Specify a memory buffer for temp-tables
# You probably want at least 1024 if you're writing 60000+ records
# Maybe larger depending on Progress version and other TT activity

-tmpbsize # block size for temp-tables, different blocks sizes have pros & cons

TomBascom · Jan 14, 2009

"lr" should not be zero. You need to debug that.

FrancoisL · Jan 14, 2009

Changing this

Code:

FOR EACH wo_mstr NO-LOCK WHERE 
    wo_due_date >= data_u  AND wo_rel_date >= data_u AND wo_rel_DATE <= data_u1 

                           AND   wo_cc >= buyer1 AND wo_cc <= buyer2
                            AND wo_part >= part1 AND wo_part <= part2
                            AND (wo_status = "R"),
                            EACH wod_det WHERE wod_lot = wo_lot USE-INDEX wod_det NO-LOCK,
                            EACH pt_mstr WHERE pt_part = wod_part AND pt_pm_code <> "P" USE-INDEX pt_part:

to this

Code:

FOR EACH wo_mstr NO-LOCK USE-INDEX wo_due_part
WHERE      wo_due_date >= data_u  
AND wo_part >= part1 AND wo_part <= part2
                             AND wo_rel_date >= data_u 
AND wo_rel_DATE <= data_u1 
AND   wo_cc >= buyer1 AND wo_cc <= buyer2
                            AND (wo_status = "R") ,
                            EACH wod_det WHERE wod_lot = wo_lot USE-INDEX wod_det NO-LOCK,
                            EACH pt_mstr WHERE pt_part = wod_part AND pt_pm_code <> "P" USE-INDEX pt_part:

Will yield better results especially when you are querying specific part ranges.

Not optimal code

domel

New Member

sdjensen

Member

Casper

ProgressTalk.com Moderator

domel

New Member

sdjensen

Member

domel

New Member

Casper

ProgressTalk.com Moderator

parul

Member

domel

New Member

TomBascom

Curmudgeon

domel

New Member

TomBascom

Curmudgeon

TomBascom

Curmudgeon

TomBascom

Curmudgeon

FrancoisL

Member