Memory leak? 4

[andjem1]

Hi everyone. I have an app that converts values in a text file (cnc gcode) displayed in a Richedit. The program works ok except that every time the program is run there is a noticeable drop in speed. Also the larger the file the worse the problem. i.e. 61000 lines take 6mins 30s and 87000 take 13mins 10s.

I suspect a memory leak somewhere, however I don't really understand what they are, which part of the code is responsible and what to do about it.

The 'guts' of the program are shown below with the two main functions. (thanks to whosrdaddy)
If someone could point me in the right direction I would appreciate it.
Thanks
Beginner (as if you hadn't guessed!) using the free Turbo Delphi.

procedure TForm1.btnRunClick(Sender: TObject);
var
   I, LineCount: Integer;
   OldValue, Line : String;
   Radius, Diameter : Extended;
    begin
      If HasRun = 1 Then
        begin
         Beep;
         Showmessage('Code is already converted. You must reopen file to convert with another setting.');
         exit;
        end
      else
         Radius := StrToFloat(EditRadius.Text);  // Get the Stock radius from user.
         Diameter := StrToFloat(EditRadius.Text) * 2;  // Convert to Diameter
         Linecount := Richedit1.Lines.count;  // save the amount of lines to variable 'Linecount'
         btnRun.Caption := 'Calculating'; // Change caption of Run button.

        for i := 0 to Richedit1.Lines.Count-1 do  //from line 0 to total number of lines less 1 do the following.
         begin
           Line := Richedit1.Lines[i]; // load current line which is a string to variable 'Line'
           //X to B conversion type
           if radioButton1.Checked = true then
             if ansicontainsstr (Line,'X') then
                if HasUnwanted(['Y','Z','F','H','D','S','M'],Line) then
                  begin
                  OldValue := GetXMove(Line);
                  NewValue := 'B' + Convert(OldValue,Radius);
                  end
                else
                  begin
                  OldValue := AnsiMidstr(Line,(ansipos('X',Line)),maxint);
                  NewValue := 'B' + Convert(OldValue,Radius);
                  end;
            //Y to A conversion type
            if radioButton2.Checked = true then
              if ansicontainsstr (Line,'Y') then
                if HasUnwanted(['Z','F','H','D','S','M'],Line) then
                  begin
                  OldValue := GetYMove(Line);
                  NewValue := 'A' + Convert(OldValue,Radius);
                  end
                else
                  begin
                  OldValue := AnsiMidstr(Line,(ansipos('Y',Line)),maxint);
                  NewValue := 'A' + Convert(OldValue,Radius);
                  end;
           //X to A conversion type
           if radioButton3.Checked = true then
             if ansicontainsstr (Line,'X') then
                if HasUnwanted(['Y','Z','F','H','D','S','M'],Line) then
                  begin
                  OldValue := GetXMove(Line);
                  NewValue := 'A' + Convert(OldValue,Radius);
                  end
                else
                  begin
                  OldValue := AnsiMidstr(Line,(ansipos('X',Line)),maxint);
                  NewValue := 'A' + Convert(OldValue,Radius);
                  end;
            //Y to B conversion type
            if radioButton4.Checked = true then
              if ansicontainsstr (Line,'Y') then
                if HasUnwanted(['Z','F','H','D','S','M'],Line) then
                  begin
                  OldValue := GetYMove(Line);
                  NewValue := 'B' + Convert(OldValue,Radius);
                  end
                else
                  begin
                  OldValue := AnsiMidstr(Line,(ansipos('Y',Line)),maxint);
                  NewValue := 'B' + Convert(OldValue,Radius);
                  end;
             NewValue := AnsiReplaceStr(Line,(OldValue), (NewValue)); //Swap the old X or Y value with the new A or B value.
             Richedit1.Lines[i] := NewValue; // write converted line back into memo
             Progressbar1.Position := Trunc(i/Linecount*105);
     end;
          //Apply comments to the start of the converted program.
           richedit1.selstart := 0;
           richedit1.seltext := '(PROGRAM CONVERTED WITH WRAPPER)'+#13#10;
           richedit1.seltext := '('+ DateTimeToStr(Now) + ')'+#13#10;
             if radioButton1.Checked = true then
               begin
               richedit1.seltext := '(X AXIS MOVES CONVERTED TO ROTATIONAL B AXIS)'+#13#10;
               end;
             if radioButton2.Checked = true then
               begin
               richedit1.seltext := '(Y AXIS MOVES CONVERTED TO ROTATIONAL A AXIS)'+#13#10;
               end;
             if radioButton3.Checked = true then
               begin
               richedit1.seltext := '(X AXIS MOVES CONVERTED TO ROTATIONAL A AXIS)'+#13#10;
               end;
             if radioButton4.Checked = true then
               begin
               richedit1.seltext := '(Y AXIS MOVES CONVERTED TO ROTATIONAL B AXIS)'+#13#10;
               end;
     if i = Linecount then
       begin
        if radioButton1.Checked = true then
           showmessage('          X to B axis'+#13#10 +'conversion complete.');
        if radioButton2.Checked = true then
           showmessage('          Y to A axis'+#13#10 +'conversion complete.');
        if radioButton3.Checked = true then
           showmessage('          X to A axis'+#13#10 +'conversion complete.');
        if radioButton4.Checked = true then
           showmessage('          Y to B axis'+#13#10 +'conversion complete.');
      end;
      btnRun.Caption := 'Conversion Complete';
      HasRun := 1;   //Setting HasRun variable to 1 prevents converting the code twice.
      EditRadius.Enabled := False; // Code is already converted so can't be run again.
end;

function GetXMove(Str : string) : String;
var
    Index : Integer;
    Ch : Char;
    StrLen, XPos : Integer;
    NumStr : string;
         begin
              Result := '';               // Make sure result is empty first.
              StrLen := Length(Str);      // Get the length of the string passed to the function.
            if StrLen = 0 then Exit;      // Sanity Check
              Index := 0;
              NumStr := '';
              XPos := ansipos('X',Str)-1; // get position of X in the string
              Index := XPos;              // Give X position to Index.
            repeat
              Inc(Index);                 // Increment Index. Move along the string 1 position every repeat
              Ch := Str[Index];           // At every position get that character and pass it to variable ch
            case Ch of         // valid char. Ch is checked against wanted chars.
              'X','-','0'..'9','.' : NumStr := NumStr + Ch; //every wanted char is added to string held in variable 'NumStr'
          else
            begin
              Result := NumStr;
              Index := MaxInt;
            end;
        end;
            until
              Index >= StrLen;
end;

Function Convert (Value:string; var radius : extended) :string;
var
   Numbers, angle: Extended;
   formated: String;
   begin
     result := '';
       numbers := strtofloat (ansimidstr (value, 2, maxint));
       Angle := (numbers)*180/Pi/(Radius);
       Formated := Format('%.3f', [Angle]); //Format angle to 3 decimal places.
       Result := Formated;
end;

 

[Glenn9999]

A memory leak is what happens when a piece of memory is obtained by the program but not freed. From what I read in your question, your concern is more performance, am I right?

Regardless, we can figure out something. Please clarify your concern.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[andjem1]

Thanks for the reply. Yes performance, or lack of, is my concern. If I convert a file with 20,000 lines it takes about 30 seconds so I would hope that a file with 60,000 lines will take 3 times as long, yet it actually takes about 6mins 30s and a file with over 100,000 lines seems to lock up the program altogether. I also notice a slowing down of the progress bar as it moves across almost as if the program is getting choked somehow.
So my question is, is my code or structure inherently inefficient? have I tried to do too much with one procedure? and is there a way to speed up the process of converting each line in turn?

Just so you can see what the program does, here's a couple of short before and after extracts of gcode. This example converts the Y values to A values. The program offers four types of conversion. X to A, X to,B, Y to A and Y to B.
Before conversion
N40 G0X0.000Y0.000S16000M3
N50 G0X-25.000Y-30.000Z31.000
N60 G1Z22.500F762.0
N70 G1Y30.000F2540.0
N80 X-24.997Y30.359

After conversion
N40 G0X0.000A0.000S16000M3
N50 G0X-25.000A34.377Z31.000
N60 G1Z22.500F762.0
N70 G1A-34.377F2540.0
N80 X-24.997A-34.789

In fact you can see the actual program called Wrapper here

http://vectric.com/forum/viewtopic.php?t=2920

if you want.

 

[Glenn9999]

Okay, I've been looking at the bits of code you posted and having the app answers a few of the questions I had in looking at the code.

Anyway, to start:

If I convert a file with 20,000 lines it takes about 30 seconds so I would hope that a file with 60,000 lines will take 3 times as long, yet it actually takes about 6mins 30s and a file with over 100,000 lines seems to lock up the program altogether

Don't assume all code is linear. Depending on the algorithm in question, it can be just about anything. In fact, the computer scientists have a term for it called Big-O. It's time of execution expressed in an algebraic function in terms of the amount of data. For example, a bubblesort is O(n squared), which means that the execution time increases exponentially given more records. If 400 records takes 10 seconds, 800 records takes 100 seconds, etc. Your testings have obviously proved that your code is not linear.

So my question is, is my code or structure inherently inefficient?

For what you posted of it, I saw one or two things that were obvious to me that could be done to help matters. Of course there could be many more, but we'll have to investigate those things in turn.

have I tried to do too much with one procedure?

This is part of the first step. The TForm1.btnRunClick was rather obfuscated to the point that I ended up copying the code you posted into a project, and splitting it up so I could see the logic a bit better (and throwing various controls that were suggested onto the form as well as dummy missing functions so it would compile). But the basic first step would be to try to clean things up a bit.

and is there a way to speed up the process of converting each line in turn?

Sure. We'll have to see what is available and what could be done, but the first step would be to clean things up so the logic is expressed in simpler terms.

First question I had:

 If HasRun = 1 Then
    begin
      Beep;
      Showmessage('Code is already converted. You must reopen file to convert with another setting.');
      exit;
    end
  else
    Radius := StrToFloat(EditRadius.Text);  // Get the Stock radius from user.
    Diameter := StrToFloat(EditRadius.Text) * 2;  // Convert to Di

In this section, do you mean the "else" to apply to all the subsequent code, or to the line directly after it (as the compiler logic will do)? If HasRun is 1, all the code after the line after else will run anyway. From the logic it seems you intend the process to run only if HasRun is anything but 1, so the else would be unnecessary. Unless a "begin" went missing somewhere.

First item to think about: When I split up the code, I did it by taking the code in your for loop and putting it into a separate procedure. So I ended up with:

    for i := 0 to Richedit1.Lines.Count-1 do  //from line 0 to total number of lines less 1 do the following.
      DoLine(i);

in the btnRunClick, and the code you had there with accompanying local variables in the new procedure.

procedure TForm1.doline(i: integer);

That would be a good thing to start with in cleaning up the code - it made it much easier to understand and see what is going on. (BTW, I might suggest things we might undo in part later)

Anyhow, this post is getting long for now, so the first step after making that change to see things a little better is to get a number on how much time your code is taking to come up in front of you so you have an idea of the impact of any changes that would be made.

To be simple, this definition can be copied to return a system time.

function TimeMS: DWord;
          stdcall; external 'winmm.dll' name 'timeGetTime';

To use it, define a DWord value in memory, then do in your code like this:

time := TimeMS;
{ all the code you want to time here }
time := TimeMS - time;

Then when you're done, "time" will be the elapsed milliseconds.

(this will be a good start for you to get going on optimizing this, I'll start writing some other cleanup things I saw in the next post)

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[Duoas]

I don't see any obvious memory leaks.

I think the biggest time killer is using a tRichEdit through the VCL, which is notoriously slow.

Life would go much faster if you do something like use a memory-mapped file and write the tRichEdit.plaintext to it, then scan the mmfile for your data. Or...

Use SendMessage to talk directly with the Rich Edit (and avoid the VCL).

Hope this helps.

 

[Glenn9999]

The main focus of what I mean by "cleanup" is to express the logic in the simplest of terms. Meaning, remove things that are unnecessary (logic, variables), things that are duplicated, etc. If logic absolutely requires A, B, and C, we don't need to be doing D, or doing C twice, etc.

Of course, since I don't have the project here I don't know if any of these things will make an improvement (or will even work given your logic), but logically I'm thinking they should make a difference, especially in your huge file runs.

Always make a backup of your code at each step so you can easily go back if you find you mess something up or you find that the change pushed the time worse instead of better. As I reiterate, none of this is tested, so be cautious as you go and back up.

First change
There's really no need to copy the line array reference into line. Logic that's not really serving anything. So we remove:

Line := Richedit1.Lines[i];

And change all the references of "line" to "Richedit1.Lines".

Second Change
Making this change shows us something very important to know for logic. In fact, it makes it stick out like a sore thumb. You ask several questions in your logic repeatedly in the loop, whose answers will never change in the logic in the course of the loop. For example, if radiobutton2 is checked, it's always going to be checked each and every time the loop executes. This means, they need to go out of the loop.

So I change "procedure TForm1.doline(i: integer);" to "procedure TForm1.doline;" and then move the loop logic into the procedure.

I notice in DoLine that there are four distinct questions, so I'll just show you one of them, changed.

    //X to B conversion type
    if radioButton1.Checked = true then
       for i := 0 to Richedit1.Lines.Count-1 do
         begin
           if ansicontainsstr(Richedit1.Lines[i],'X') then
              if HasUnwanted(['Y','Z','F','H','D','S','M'],Richedit1.Lines[i]) then
                begin
                  OldValue := GetXMove(Richedit1.Lines[i]);
                  NewValue := 'B' + Convert(OldValue,Radius);
                end
              else
                begin
                  OldValue := AnsiMidstr(Richedit1.Lines[i],(ansipos('X',Richedit1.Lines[i])),maxint);
                  NewValue := 'B' + Convert(OldValue,Radius);
                end;
           NewValue := AnsiReplaceStr(Richedit1.Lines[i],(OldValue), (NewValue)); //Swap the old X or Y value with the new A or B value.
           Richedit1.Lines[i] := NewValue; // write converted line back into memo
           Progressbar1.Position := Trunc(i/Linecount*105);
         end;

As you can see, the IF statement asking about whether the radiobutton1 is checked is now moved outside of the loop. Therefore it will only be executed once and exactly once. You should find a time savings. Be sure to repeat this on all 4 of your questions, and be sure to move/fix the last 2 lines so they occur at the end of the loop as was shown so your lines are updated.

This ought to be a very good start. Please let me know if these things are working out and if they made a difference. Then if you wish we can go on with a few more changes & descriptions to clean the logic up some.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[Duoas]

Hey Glenn9999, good suggestions. However, your first change, while logically consistent, plays into the weakness in the VCL tRichEdit component that I mentioned above.

Each and every time that Richedit1.Lines is accessed sets in motion a great deal of overhead that is a significant factor in the algorithm's speed.

Copying it into a temporary value accomplishes much the same thing as moving logic references out of the loop does: only one time-intensive operation occurs per loop instead of many. (Remember, Delphi strings are reference-counted, so this is essentially one dereference, one increment, and one dword variable assignment.)

Hope this helps.

 

[Glenn9999]

Hey Glenn9999, good suggestions. However, your first change, while logically consistent, plays into the weakness in the VCL tRichEdit component that I mentioned above.

Definitely. Sometimes, though, it helps to see what is going on. He could undo the first change and be good on the second. Much of performance tuning involves trial and error. He might find that it might do some help, it might not. Much that could be typed on the topic, that's why I was trying to piecemeal it - replacing around 240000 compares with 4 ought to get him some speed improvement.

I think if we get the chance to see the end of this (I'm going to suggest he run a profiler against it once all the obvious stuff is out of the way), we'll probably find that most of the time is being taken in string manipulation (Ansi*).

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[andjem1]

First of all, a big thanks to you both for taking an interest in my little app. I'll report on your recommendations as best as I can in the order I understood them.

From the logic it seems you intend the process to run only if HasRun is anything but 1, so the else would be unnecessary.

Yep that 'else' was a mistake.

Then when you're done, "time" will be the elapsed milliseconds.

I have not come across a DWord before, using your code I can now display the elapsed time in a label. I think this will prove very useful.

For example, if radiobutton2 is checked, it's always going to be checked each and every time the loop executes. This means, they need to go out of the loop.

Yes I see that now. I've made the change to all four distinct questions it seems so obvious when pointed out.

And change all the references of "line" to "Richedit1.Lines".

I made this change, but I must be doing something wrong here at the moment. When I convert a small test file all the gcode is deleted so I have gone back to using 'Line'. The odd thing is, the timer showed a slight increase in the elapsed time.

I haven't quite got my head around splitting up the code and using the Doline procedure yet. Ill continue to look at that.

Duoas, the concept of using a "memory-mapped file" is way beyond me for the time being, I think I'm only just beyond the "Hello World" stage.

Anyway, the result of rearranging the loops with a 20,000 line file, time is reduced from 35360 milliseconds to 34897. Not a huge difference but its moving in the write direction.

 

[Glenn9999]

so I have gone back to using 'Line'. The odd thing is, the timer showed a slight increase in the elapsed time.

That's okay, if the timer showed an increase in the time, that means that you went backwards, and therefore the change wouldn't have been a good idea. Trial and error. One victory, one defeat so far, no problems.

I haven't quite got my head around splitting up the code and using the Doline procedure yet. Ill continue to look at that.

Not really that necessary if you can get your mind around the code you have at the moment. Actually at the point I got the code to when I was playing with it, I have it split in a totally different way now that's more understandable to me anyway. Trial and error, again.

Duoas, the concept of using a "memory-mapped file" is way beyond me for the time being, I think I'm only just beyond the "Hello World" stage.

http://en.wikipedia.org/wiki/Memory-mapped_file

The concept is not a bad one, depending on how you are reading the file (you don't show us that). Any way you can get the least I/O (either from reading the file, or from keeping your program from hammering the page file) is a good thing.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[Glenn9999]

Okay, a third change. Again it may make a difference, it may not. Something for you to try, again.

In your calculations, I notice there are several constants going into the code that could be pre-calculated beforehand. I'll write some examples and you can take it from there:

Progressbar1.Position := Trunc(i/Linecount*105);

This is done once per time in the loop. There's a variable there, and that's fine. But LineCount and 105 are constants going into the loop, so we don't need to be calculating it there. Using some algebraic transitiveness given the order of operations, you should be able to precalculate it.

i/Linecount*105 =
i * (1/Linecount) * 105 =
i * (105 / LineCount).

You should be able to put the result of 105/LineCount into an extended variable before the loop starts and see another performance improvement. Again YMMV. Even better here would be to set the ProgressBar bound (Max) to the line count, and then use "i" as your Progressbar position (no calculation at all!).

But a more relevant one out of your Convert function - it gets called multiple times in the loop so it becomes a concern to optimize:

Angle := (numbers)*180/Pi/(Radius);

Numbers and radius are variable, but 180/Pi is a constant.

Numbers*(180/Pi)/Radius =
(Numbers / Radius) * (180/Pi)

So we can rework this algebraically and define 180/Pi to be a hard-coded constant calculated upon compilation as opposed to each time in the loop as it is run.

const
  Pi180: Extended = (180 / Pi);

Angle := (Numbers / Radius) * Pi180;

That finishes the obvious things that I saw that would help. I'll post about running the profiler against it (like I mentioned before) soon.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[Glenn9999]

Here's a Delphi profiler you can run against your program. If everything works as it should, the program should be able to point out to you where your program is taking the most time.

Eric Grange's Sampling Profiler

The first step is that you'll need to go into Project/Options/Linker and click on "Include TD32 Debug Info". Then compile and save your project and exit Delphi.

When you start up the profiler, click on the little folder next to Application Name, and find the EXE of your project and select it. When it comes back, it should say "TD32 Information found" below it.

Then press the "Play button" (green triangle) third from the left on the bar. It should then run your application. Feed it one of your data files (the bigger the better, as much as you can tolerate), and then let it run. When your program completes close it out.

The profiler should then produce a graph that shows a list of different DLL calls and percentages of time. One of them should be your app. You should be able to click the + next to it and it will give you a breakdown from there based on different units you have compiled and then should show you what is taking up the most time within your program by function. I have a hard time getting it to find the source files, but if you can get it to do that, it should show you source lines to look at, as well.

Hope that helps, and please post back the results. We can see where you could look from there.


----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[Duoas]

Delphi automatically folds constants in algebraic expressions. So you won't find any need to move the 180/Pi out of the loop or anything...

 

[Glenn9999]

Delphi automatically folds constants in algebraic expressions.

Even when they're connected to variables? I knew it did that for simple constant sequences (i.e. var1 := 3*2/6; would become var1 := 1 in the code), but for many compilers I've dealt with, a lot depended on the order of operations on how that was done. If I did var2 := var1/6*var3/2;, it wouldn't do it because the variable appeared first, and therefore all intermediate results were considered a produced variable result.

I haven't been able to test anything time-wise to know the difference, so I could be wrong, of course.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[DjangMan]

Just to throw my two cents - don't update the progress bar so often. Drawing graphics on the screen is probably the slowest part of the whole process. I had an application that would process 40000 records in 40 minutes with a progress bar. I added a timer to update the progress bar every half-second instead of in my main loop and my processing time went down to 10 minutes - no other change in code.

 

[Duoas]

Even when they're connected to variables?

Yep. It's pretty smart.

Everything that's a constant (literals and const) will be changed into a single value (as possible).
Then a simple loop analysis will eliminate variables with static value.

It isn't perfect, of course, so you are right to suggest always testing (and as you explicitly indicated, just to put such things outside the loop anyway...)

Just to throw my two cents - don't update the progress bar so often.

Quoted for agreement.

 

[Glenn9999]

Okay, I got the chance to take your code and the data posted here and come up with a working version of my own to play with.

I profiled the thing too to start into the not-so-obvious suggestions. You always learn things in profiling and I definitely learned one thing. Activities involved in TRichEdit take up about 84% of the time in the program when you run 20000 lines of data in it (score one, Duoas, star for you from me!). Which means it's got to go if you habitually plan on running big files through your program.
(Maybe first step in the future, though your program did need other things?)

Typically in data processing programs, which yours is, people prompt for input and output files, and then let the program process the data without it showing up on a control.

Some times:

Original Loading File (20000 recs): 64170ms.
Original File Processing Time (20000 recs): 92755ms.
Taking TRichEdit out of the loop (20000 recs): 2315ms.

The third one is changed, so the file names are prompted and then processed upon. TRichEdit is only used to put the status messages.

(probably could push that down even more, might play with it.)

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[Glenn9999]

It isn't perfect, of course, so you are right to suggest always testing

20 million records, the arithmetic change with the constant added to the code described above removed 3/4 of a second. Not too much comparatively to worry, but answered the question anyhow.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.

 

[andjem1]

I can't thank you enough for the time you have spent with this.

The concept is not a bad one, depending on how you are reading the file (you don't show us that).

Sorry, I'm loading the file into a Richedit and performing some simple functions as it comes in to simplify processing.

procedure TForm1.FileOpen1Accept(Sender: TObject);
var
     BlockCount: integer;
begin
     Tempfilename := Fileopen1.Dialog.FileName;
      if Fileexists(Tempfilename) then
       Begin
         Richedit1.Lines.LoadFromFile(Tempfilename);
         Richedit1.Text := AnsiUpperCase(richedit1.Text);// Make all text uppercase to simplify conversion functions.
         Richedit1.Text := StripComments(Richedit1.Text);// Strip out all gcode comments to simplify conversion functions.
         EditRadius.Enabled := True;
         Blockcount:= Richedit1.Lines.Count; //Count the number of lines and put the amount in Blockcount variable.
         StatusBar1.Panels[0].Text:='Number of Blocks =' + ' '+ inttostr(Blockcount); //Display block(line) count.
         HasRun := 0; // Set this value to 0 to allow conversion to run.
         Progressbar1.Position := 0;
         btnRun.Caption := 'Run Conversion';
         //Check whether G02 or G03 is in the file. Exit and clear richedit if found.
           if ansicontainsstr(richedit1.Text, 'G02') then
             begin
               Showmessage('Wrapper does not currently support conversion of arc moves (G02 or G03)');
               Richedit1.Text := '';
               exit;
             end;
           if ansicontainsstr(richedit1.Text, 'G03') then
             begin
               Showmessage('Wrapper does not currently support conversion of arc moves (G02 or G03)');
               Richedit1.Text := '';
               exit;
             end;
       End;
end;

Reading through your replies I'm now coming round to the idea that the order in which the program works could be better.
1. Prompt for input file, maybe show chosen file in a label or something.
2. Process file without loading into Richedit (something I know nothing about at the moment.)
3 Once processing done, then display file in Richedit or memo to allow user to make any manual edits before saving.

I did make the changes as suggested in "Third change" in fact I disabled the progress bar altogether but the difference in time was very small.

Anyway, you have given me much to think about and explore.

I now have two useful methods of testing changes, ideas on how to clean up and optimise my code and a whole new direction to follow.

Thanks again.

P.S. I'll let you know how I get on. Brain doesn't find this easy so it may take some time!

 

[Glenn9999]

P.S. I'll let you know how I get on. Brain doesn't find this easy so it may take some time!

I'm still playing around with what I have here. If you want I can post it when I'm done with it. It should help you somewhat.

----------
Those who work hard are rewarded with more work and remembered come time to downsize. Those who hardly work are given a paycheck and ignored completely.