Measure, refactor, measure, refactor, measure… stop… revert

As I said already, I like refactoring code. It’s an activity that is avoided by a lot of programmers. The first excuse is that it usually takes time, a lot of time. The second is that code works, it just works so no changes are needed and it is possible to introduce new bugs refactoring. The third is that there will be no real advantage to streamline the code.

I don’t want to do a post on why refactoring is important but I just want to reply to the 3 most common excuses:
1) refactoring should be done during code development and not when the product is completed
2) tests should be in place to verify how the code works before and after the refactoring
3) it really really depends. After a refactoring, code can be more clean and easy to read. Snippets of code repeated in various parts can be refactored to a function and be present only once. Refactoring is not specific to have quicker code but it is possible, as a side effect or when set as a goal.

In the previous post we had a look at a function that was coded, well, not properly.

FUNCTION record_lock( ..., wait, ... )
   PRIVATE row,col,time,video

   row = Row()
   col = Col()
   time = wait
   video = SaveScreen( 21, 0, 23, 79 )
   IF RLock()
      csrput( row, col )
      RETURN .T.
   ENDIF
   DO WHILE .T. // Loop for RLock() retries

Also the code in the loop needs some refactoring, but it is rarely used so it’s not a problem. Also the code shown here is not really a problem, since it correctly works, but when you call this function a lot of times, there is some overhead that we may avoid.

My refactoring:

FUNCTION record_lock( ..., wait, ... )
   LOCAL row,col,time,video

   IF RLock()
      RETURN .T.
   ENDIF
   row = Row()
   col = Col()
   time = wait
   video = SaveScreen( 21, 0, 23, 79 )
   DO WHILE .T. // Loop for RLock() retries

I timed the printing procedure I used as a test in the previous post, where the function is called 15600 times, before and after the code changes. From 4.85 to 4.15 seconds (timing averaged on several tests). It’s just 0.7 seconds shaved, but it’s about 14.5% quicker now. For a 2 minutes work.

I wanted to perform another test: opening the files in exclusive mode. I changed the flag used to open the database files and removed the test in the printing loop.
The average time dropped to 3.32 seconds, a saving of 31.5%
Just for a couple of changes!

I did measure before doing any changes: after the first and simplest ones I got a 14.5% speed increment. Opening the files in exclusive mode I saved 31.5% of the time but I now have to change code elsewhere because now if another instance of the program is run, it won’t work properly. Infact I opened all the files in exclusive mode but instead I should open a couple of them in exclusive and the others in shared mode. This means adding lines, keep them in order as the program expect them, fully test that the program doesn’t break if a file is found opened by another instance of the program…. and unfortunately it breaks.

The code is almost all multi-user ready (files opened in shared mode, records locked, etc) but the programmer that sent the code to me said that it shouldn’t be used in multi-user. Don’t know why, anyway these last changes break the program.

Revert the last change, back to shared mode.

I imagine it wouldn’t be a problem to wait 4.15 seconds instead of 3.5 seconds when this job took hours.

PS: it’s really difficult timing code snippets on a multi tasking operating system working on a multi core cpu, when you can’t control exactly what other programs the pc is running in that moment. Anyway, I removed the GOTO RecNo() line and I saved another 0.40 seconds.

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>