Omnis 7 Refactoring Legacy Code

Stephen Miller stephenmiller1958 at gmail.com
Tue Jan 31 05:36:19 UTC 2023


Hi Martin

Agree totally with Doug's final comments about hardware and environment.

What is the gear the application and datafile are running on?

Can you run the process on the server using VNC or whatever to cut out the
network portion?

Is the datafile auto-incrementing and is nearly full?

Is it your perception that the process is slow or the client's?

Is the process running at night?

Do the copy formats have unnecessary indexes?

Just suggesting that I would look at these issues first before touching the
code.

On Tue, 31 Jan 2023 at 04:02, Doug Easterbrook via omnisdev-en <
omnisdev-en at lists.omnis-dev.com> wrote:

> martin:
>
> you don’t say what the purpose of the routine is or what you are trying to
> accomplish.   so I reformatted it to read it.
>
> If I was to guess, it looks like
> 1) you have TWO files.   F_TRANSPAST and F_TRANSPAST2
> 2) you read all data from F_TRANS into a list
> 2) it looks like data from the list is inserted into F_TRANSPAST1 from the
> F_TRANS file
> 3) it then looks like the list is processed again and if PRD_NO=12
>   3A) you delete all data from F_TRANSPAST2
>   3b) you write all data into F_TRANPAST2
>
> if looks like F_TRANSPAST always grows in size (data is never deleted)
>
> it looks like F_TRANSPAST2 only has recent data.
>
>
>
> you don’t mention if the files containing E1_RSNNO and D1_RSNNO are joined
> in the omnis 7 connected records fashion.    SOme people used connected
> records to get relational data..
>
> but, I suspect, that they are not because you are doing single file finds
> within a loop.
>
> it means prior to each insert, you are doing two reads.      and since it
> is in two loops, you could be doing 4 reads for 10,000 records,   or 40,000
> reads that coild be unnecessary — and the only reason to do the reads
> appears to be to get fields
>  -E1_TERMINATED
> - E1_EDATE
> - E1_SUSPEND
>
> for comparisons to decide is you insert a record or not.
>
> I don’t know the file structure .. but if you were using connected
> records, then I’d build the list L_WORK1 to include the three fields I was
> comparing.   If the files are connected (a differnt topic), then omnis
> automatically reads the data from the connected files.
>
> Now you have L_WORK1 as a list with the data you are going to use to
> compare
>
> you may even be able put the 3 fields into a SEARCH criteria or change
> your set search as calculation form
>
> Set search as calculation {(T1_AMOUNT<>0.00)}
>
> to
>
>
> Set search as calculation
> {(T1_AMOUNT<>0.00)&(E1_TERMINATED=0)&((E1_EDATE='')|(E1_EDATE<PRD_ENDDATE))&(E1_SUSPEND=0)}
>
>
> if possible, then you end up
> 1) only reading the data you need once into the lsit  (and reading less of
> it, depending on how much data meets the citieria)
> 2) then you may )or may not) need the single file finds — again, that
> depends if F_TRANSPAST and F_TRANSPAST1 are also connected to the files
> referenced by E1_RSNNO and D1_RSNNO.
>
> if they are not connected, you can remove 40,000 reads from the loop and
> read less data to make this faster (assuming you have connected records on
> F_TRANS)
>
>
> depending on which of the thoughts you can implement above, you should be
> able to shave significant time from the process.     maybe 3 to 4 times as
> fast.
>
>
> —————————————————————————————————————————
> if not, you can do it another way, much like studio table class, and do
> all the record manipulation in memory before writing it … i.e
>
> 1) define LWORK1 to contain the extra fields you want to write like
>     Calculate TP_PRDNO as PRD_NO
>             Calculate TP_PRDYEAR as PRD_YEAR
>             Calculate TP_PRDMTH as PRD_MTH
>             Calculate TP_D1RSNNO as D1_RSNNO
>
> and
>  the fields
>  -E1_TERMINATED
> - E1_EDATE
> - E1_SUSPEND
>
> 2) read the data
> process the data in L_WORK1 to determine what to put into F_TRANSACT
> SAVE the RSN for TP_D1RSNNO back in L_WORK1
>
> write out F_TRANSPAST
>
> 3) loop through the list again
>
> Write out F_TRANSPAST2 using the same variables from the list and you used
> to create F_TRANS
>
> DO NOT BOTHER re-reading
>  Single file find on E1_RSNNO (Exact match) {TP_E1RSNNO}
>         Single file find on D1_RSNNO (Exact match) {E1_D1RSNNO}
>
> use the RSN from F_TRANSPAST as the RSN for F_TRANSPAST2 as below.   why
> do that….  because it looks like you delete the file F_TRANSPAST2 befire
> you fill it up again, and the RSN likely doesn’t matter.
>
> Calculate TP2_RSNNO as TP1_RSN
>
>
> that might make it twice as fast since oyu don’t need the read of the
> single file finds in the second loop.  you already have the data you need.
>
> —————————————————————————————————————————
>
> and as a final thouht, I see this structure twice
>
>         Prepare for insert with current values
>             Calculate TP_PRDNO as PRD_NO
>             Calculate TP_PRDYEAR as PRD_YEAR
>             Calculate TP_PRDMTH as PRD_MTH
>             Calculate TP_D1RSNNO as D1_RSNNO
>             Update files
>             Prepare for edit
>             Calculate TP_D1RSNNO as TP_RSN
>             Update files
>
>
> which makes me ask ‘why are you claculating TP_D1RSNNO as D1_RSNNO, then
> assigning it to TP_RSN after you update the file
>
> it makes me think that TP_RSN is the record seq number and you are copying
> it into TP_D1RSNNO afterwards.      ‘
>
> on the surface — I don’t see why you make both fields the same ..   if
> there are no other reference to TP_D1RSNNO in your program, then why edit
> the edit the file to save it again it.
>
>
>
> —————————————————————————————————————————
>
> finally,    if this database  is not on an SSD, then buy an SSD and run
> your application on an SSD.      it will automatically go many times faster.
>
> If the machine can support an m.2 style SSD, it will go even faster.
>
>
> hardware costs far less than a developer does and if time is of the
> essense, then this is a fast way to gain performance for very cheap.
>
>
>
> —————————————————————————————————————————
>
> ;  Update all amounts in F_TRANSPAST with the non-zero amounts from
> F_TRANS.
> ;  Do NOT forget to indicate the current period (i.e. period before
> closing)
> ;; >>Already picked up from F_TRANS
> Set current list L_WORK1
> Define list {T1_RSN..T1_FL1BENEC1}
> Set main file {F_TRANS}
> Set search as calculation {(T1_AMOUNT<>0.00)}
> Build list from file on T1_RSN (Use search)
>
> If #LN>0
>     Set main file {F_TRANSPAST}
>     Set current list L_WORK1
>     Redefine list {TP_RSN..TP_FL1BENEC1}
>     Set read/write files {F_TRANSPAST}
>
>     For each line in list from 1 to #LN step 1
>         Load from list
>         Single file find on E1_RSNNO (Exact match) {TP_E1RSNNO}
>         Single file find on D1_RSNNO (Exact match) {E1_D1RSNNO}
>
>         If
> (E1_TERMINATED=0)&((E1_EDATE='')|(E1_EDATE<PRD_ENDDATE))&(E1_SUSPEND=0)
>             Prepare for insert with current values
>             Calculate TP_PRDNO as PRD_NO
>             Calculate TP_PRDYEAR as PRD_YEAR
>             Calculate TP_PRDMTH as PRD_MTH
>             Calculate TP_D1RSNNO as D1_RSNNO
>             Update files
>             Prepare for edit
>             Calculate TP_D1RSNNO as TP_RSN
>             Update files
>         End If
>     End For
>
>     Set read-only files {F_TRANSPAST}
>     If PRD_NO=12
>         Set main file {F_TRANSPAST2}
>         Set read/write files {F_TRANSPAST2}
>         Delete data {F_TRANSPAST2}
>         Set current list L_WORK1
>         Redefine list {TP2_RSN..TP2_FL1BENEC1}
>             For each line in list from 1 to #LN step 1
>             Load from list
>             Single file find on E1_RSNNO (Exact match) {TP2_E1RSNNO}
>             Single file find on D1_RSNNO (Exact match) {E1_D1RSNNO}
>                 If
> (E1_TERMINATED=0)&((E1_EDATE='')|(E1_EDATE<PRD_ENDDATE))&(E1_SUSPEND=0)
>                     Prepare for insert with current values
>                     Calculate TP2_PRDNO as PRD_NO
>                     Calculate TP2_PRDMTH as PRD_MTH     ;; tp_prdmth
>                     Calculate TP2_PRDYEAR as PRD_YEAR
>                     ;  Calculate TP2_D1RSNNO as D1_RSNNO
>                     Update files
>                     Prepare for edit
>                     Calculate TP2_RSNNO as TP2_RSN
>                     Update files
>                 End If
>             End For
>         Set read-only files {F_TRANSPAST2}
>     End If
> End If
> Set current list L_WORK1
> Clear list
> Quit procedure
>
>
> Doug Easterbrook
> Arts Management Systems Ltd.
> mailto:doug at artsman.com
> http://www.artsman.com
> Phone (403) 650-1978
>
> > On Jan 30, 2023, at 1:45 AM, Martin Obongita via omnisdev-en <
> omnisdev-en at lists.omnis-dev.com> wrote:
> >
> > Hi Omnis Classic funs,
> > There is this code in omnis library that is too slow when running 10,000
> records.It takes 5 hours to complete.
> > ;  Update all amounts in F_TRANSPAST with the non-zero amounts from
> F_TRANS.;  Do NOT forget to indicate the current period (i.e. period before
> closing)     ;; >>Already picked up from F_TRANSSet current list L_WORK1
> > Define list {T1_RSN..T1_FL1BENEC1}Set main file {F_TRANS}
> > Set search as calculation {(T1_AMOUNT<>0.00)}Build list from file on
> T1_RSN (Use search)
> >
> > If #LN>0Set main file {F_TRANSPAST}Set current list L_WORK1Redefine list
> {TP_RSN..TP_FL1BENEC1}Set read/write files {F_TRANSPAST}
> >
> > For each line in list from 1 to #LN step 1Load from listSingle file find
> on E1_RSNNO (Exact match) {TP_E1RSNNO}Single file find on D1_RSNNO (Exact
> match) {E1_D1RSNNO}
> >
> > If
> (E1_TERMINATED=0)&((E1_EDATE='')|(E1_EDATE<PRD_ENDDATE))&(E1_SUSPEND=0)Prepare
> for insert with current valuesCalculate TP_PRDNO as PRD_NOCalculate
> TP_PRDYEAR as PRD_YEARCalculate TP_PRDMTH as PRD_MTHCalculate TP_D1RSNNO as
> D1_RSNNOUpdate filesPrepare for editCalculate TP_RSNNO as TP_RSNUpdate files
> > End IfEnd For
> >
> > Set read-only files {F_TRANSPAST}
> > If PRD_NO=12Set main file {F_TRANSPAST2}Set read/write files
> {F_TRANSPAST2}Delete data {F_TRANSPAST2}Set current list L_WORK1Redefine
> list {TP2_RSN..TP2_FL1BENEC1}
> > For each line in list from 1 to #LN step 1Load from listSingle file find
> on E1_RSNNO (Exact match) {TP2_E1RSNNO}Single file find on D1_RSNNO (Exact
> match) {E1_D1RSNNO}
> > If
> (E1_TERMINATED=0)&((E1_EDATE='')|(E1_EDATE<PRD_ENDDATE))&(E1_SUSPEND=0)Prepare
> for insert with current valuesCalculate TP2_PRDNO as PRD_NOCalculate
> TP2_PRDMTH as PRD_MTH     ;; tp_prdmthCalculate TP2_PRDYEAR as PRD_YEAR;
> Calculate TP2_D1RSNNO as D1_RSNNOUpdate filesPrepare for editCalculate
> TP2_RSNNO as TP2_RSNUpdate filesEnd IfEnd ForSet read-only files
> {F_TRANSPAST2}End IfEnd IfSet current list L_WORK1Clear listQuit procedure
> > Is there a way I can make it run faster?
> >
> > Kind regardsMartin.
> > _____________________________________________________________
> > Manage your list subscriptions at https://lists.omnis-dev.com
> > Start a new message -> mailto:omnisdev-en at lists.omnis-dev.com
>
> _____________________________________________________________
> Manage your list subscriptions at https://lists.omnis-dev.com
> Start a new message -> mailto:omnisdev-en at lists.omnis-dev.com
>


-- 
Kind Regards,

Stephen Miller


More information about the omnisdev-en mailing list