Help with a Delphi program

[MDYarma]

I know Delphi is not really the best but.... well, I started this project in it and... didn't feel like changing. Anyway, I know very little Delphi (and very little of anything else too... but let's not beat about the bush) and this program runs slower than I'd like, so if anybody was kind enough I'd like to have some councel in order to make it faster or, at least, better in some other sense:

Code: Select all

type
  TMatrix = array [0..5119] of array [0..8191] of Boolean;

Code: Select all

const
  topex:integer = 1024;
var
  Form1: TForm1;
  done3, Dec:boolean;
  Step, mode: shortint;
  arxiu1, arxiu2: File of Byte;
  lvl, bitnorm, size:integer;
  cadena, nom1, nom2, outbox, oksent, file2sent, file1sent, errorsent:string;
  UNIVERS, PRIMIVERS, NOUVERS, buffer:TMatrix;
  pas1, pas2, pas3, pas4:array [0..5242879] of Byte;


Code: Select all

procedure TForm1.ConwayGame;
  var
  normes:array [0..9] of Boolean;
  kx, ky, suma:integer;
  a, i, j, k, l, px, py, topey, tx, ty, p1:LongInt;
  center, diagonal:boolean;
  cella1, cella2:byte;
  arxiu3, arxiu4: File of Byte;

  begin
  if FileSize(arxiu1)>FileSize(arxiu2) then size:=FileSize(arxiu1) else size:=FileSize(arxiu2);
  topey:=(size div topex);
  if (size mod topex)<>0 then topey:=topey+1;
  p1:=topey div 10;
  If p1=0 then p1:=1;
  ProgressBar1.Max:=(11*lvl)+76;

  tx:=topex*8;
  ty:=topey;
  ProgressBar1.StepIt;

  for j:=0 to ((topex*topey)-1) do
    begin
    pas1[j]:=0;
    pas2[j]:=0;
    pas3[j]:=0;
    pas4[j]:=0;
    if (j mod (p1*topex))=0 then
      begin
      StepBar;
      end;
    end;


  if (mode mod 2)=1 then center:=false else center:=true;
  if ((mode=1) OR (mode=2)) OR ((mode=5) OR (mode=6)) then diagonal:=false else diagonal:=true;

  a:=0;
  while not Eof(arxiu1) do
    begin
    Read(arxiu1, pas1[a]);
    a:=a+1;
    if (a mod (p1*topex))=0 then
      begin
      StepBar;
      end;
    end;
  CloseFile(arxiu1);

  a:=0;
  while not Eof(arxiu2) do
    begin
    Read(arxiu2, pas2[a]);
    a:=a+1;
    if (a mod (p1*topex))=0 then
      begin
      StepBar;
      end;
    end;
  CloseFile(arxiu2);

  for j:=0 to topey-1 do
    begin
    for k:=0 to topex-1 do
      begin
      cella1:=pas1[(j*topex)+k];
      cella2:=pas2[(j*topex)+k];
      for l:=7 downto 0 do
        begin
        if ((cella1 mod 2)=1) then PRIMIVERS[j][(k*8)+l]:=True else PRIMIVERS[j][(k*8)+l]:=False;
        if ((cella2 mod 2)=1) then UNIVERS[j][(k*8)+l]:=True else UNIVERS[j][(k*8)+l]:=False;
        cella1:= (cella1 shr 1);
        cella2:= (cella2 shr 1);
        end;
      end;
    if (j mod p1)=0 then
      begin
      StepBar;
      end;
    end;
  for i:=0 to 9 do
    begin
    if (BitNorm mod 2)=1 then normes[i]:=True else normes[i]:=False;
    BitNorm:=BitNorm shr 1;
    end;


  if lvl>0 then
    begin
    for i:=1 to lvl do
      begin
      for j:=0 to ty-1 do
        begin
        for k:=0 to tx-1 do
          begin
          NOUVERS[j, k]:=False;
          buffer[j,k]:=False;
          end;
        end;
      for j:=0 to ty-1 do
        begin
        for k:=0 to tx-1 do
          begin
          suma:=0;
          for ky:=1 downto -1 do
            begin
            py:=j+ky;
            if py<0 then py:=py+ty;
            if py>(ty-1) then py:=py-ty;
            for kx:=-1 to 1 do
              begin
              px:=k+kx;
              if px<0 then px:=px+tx;
              if px>(tx-1) then px:=px-tx;
              if UNIVERS[py, px] then
                begin
                suma:=suma+1;
                if (((kx=0) AND (ky=0)) AND (NOT(center))) then
                  begin
                  suma:=suma-1;
                  end;
                if ((NOT(kx*ky=0)) AND (NOT(diagonal))) then
                  begin
                  suma:=suma-1;
                  end;
                end;
              end;
            end;
          for l:=0 to 9 do
            begin
            if ((normes[l]) AND (suma=l)) then buffer[j,k]:=True;
            end;
          NOUVERS[j,k]:=(buffer[j,k] XOR PRIMIVERS[j,k]);
          end;
        if (j mod p1)=0 then
          begin
          StepBar;
          end;
        end;
      PRIMIVERS:=UNIVERS;
      UNIVERS:=NOUVERS;
      end;

    for j:=0 to topey-1 do
      begin
      for k:=0 to topex-1 do
        begin
        cella1:=0;
        cella2:=0;
        for l:=0 to 7 do
          begin
          if PRIMIVERS[j][(k*8)+l] then cella1:=cella1+1;
          if UNIVERS[j][(k*8)+l] then cella2:=cella2+1;
          if l<>7 then
            begin
            cella1:=cella1 shl 1;
            cella2:=cella2 shl 1;
            end;
          end;
        pas3[(j*topex)+k]:=cella1;
        pas4[(j*topex)+k]:=cella2;
        end;
      if (j mod p1)=0 then
        begin
        StepBar;
        end;
      end;

    if Dec then
      begin
      Delete(nom1,(Length(nom1)-3), 4);
      AssignFile(arxiu3,(nom1));
      end
    else AssignFile(arxiu3,(nom1+'.XPW'));
    Rewrite(arxiu3);

    for i:=0 to ((topey*topex)-1) do
      begin
      Write(arxiu3, pas3[i]);
      if (i mod (p1*topex))=0 then
        begin
        StepBar;
        end;
      end;
    CloseFile(arxiu3);

    if Dec then
      begin
      Delete(nom2,(Length(nom2)-3), 4);
      AssignFile(arxiu4,(nom2));
      end
    else AssignFile(arxiu4,(nom2+'.XPT'));
    Rewrite(arxiu4);

    for i:=0 to ((topey*topex)-1) do
      begin
      Write(arxiu4, pas4[i]);
      if (i mod (p1*topex))=0 then
        begin
        StepBar;
        end;
      end;
    CloseFile(arxiu4);
    end;
  end;

(I pasted only the relevant parts, there's a big part left out, but its importance is minimum, I think)

EDIT:
I forgot to write what's the point of the program ^^'' and of course you won't know if it does its work well if you don't know what's its work supposed to be. I'm sure this would be a rather crappy fry pan.....

It takes two files (of whatever kind) and makes a matrix of booleans with them, that is, all of the bits of each file are spread in order in two different matrix. Then we play around with the matrix, making them change (following something similar to this and that) and at the end we save the two matrix as two normal files, converting the matrix of booleans in the series of bits again.

I find specially stupid and annoying the loss of time that supposes
File-->File of Bytes-->Array of Bytes-->(weird loop)-->Matrix of Booleans (and vice versa)
but I know no other way to do it ^^'' even though I'm sure there's gotta be an easier way because that's a very stupid one to do something so simple.

Also the playing with the matrix is quite slow, but I'm not so sure that can be improved much.

[midnite_cowboy]

First of all, you should REALLY need to refactor your code. Refactoring means spliting you code into smaller units (functions/procedures/methods/subroutines, etc..) each with a clear defined purpose, ie "Load Matrix From File". That will make your code A WHOLE LOT more easy to understand. You may think "I know exactly what it does", yeah, but come back in 3 months and see if you can find out or ask someone else to take a look...
About clean coding practices, I recommend you the book Clean Coder, by Robert Martin. It's fun to read, clear and consice, even if you aren't an experienced programmer.

About speeding up your code, you should REALLY learn how to use a Profiler, which is an utility to find a lot about how your code is executing (number of invocations and CPU time of each subroutine, memory usage, etc) because most of the time what's bogging your program down isn't what you think (subroutine invocation, type conversion, device access, etc), and the profiler will help you find what it is.

Ok, about your program...
I see you've got lots of thing that look like this:
for i:=0 to M do
for j:=0 to K do
for l:=0 to P do
//do something

Those are called "nested loops" and can be VERY slow if the number of iterations in each is large, because each level of nesting multiplies the number of iterations. In the example, if M,K and P are 1000, there will be 1000*1000*1000 iterations.
So, first of all, try to cut down the levels of nesting. Even a two-level nested loop should raise be an alarm in your head.
Then, try to cut the number of iterations of each individual loop.
See if you can merge two o more loops in one.

Hope it helps
Cheers

[MDYarma]

Okay.... first part, agreed. ^^ I just ehm, because I have already many other subthings a part from the one I pasted it already seemed like too many to me... but, obviously weren't, I just am not used to work with it. Thanks, I'll surely try to do so.

Hm, profiler, never heard of. Okay, I take note.

I get what a nested loop is and I understand how it works (okay, not really; but, I mean, it's a loop inside a loop. they multiply eachother the times and all, that far I get. Nothing about memory handling or whatever, though) still, I see no way to avoid most of them. I'll take another careful look, I'll probably be able to fuse some of them, when I work with first one matrix and then the other, for example. Still, I see no way to escape from the big level of nesting I have in this part:

Code: Select all

for i:=1 to lvl do
      begin
      for j:=0 to ty-1 do
        begin
        for k:=0 to tx-1 do
          begin
          NOUVERS[j, k]:=False;
          buffer[j,k]:=False;
          end;
        end;
      for j:=0 to ty-1 do
        begin
        for k:=0 to tx-1 do
          begin
          suma:=0;
          for ky:=1 downto -1 do
            begin
            py:=j+ky;
            if py<0 then py:=py+ty;
            if py>(ty-1) then py:=py-ty;
            for kx:=-1 to 1 do
              begin
              px:=k+kx;
              if px<0 then px:=px+tx;
              if px>(tx-1) then px:=px-tx;
              if UNIVERS[py, px] then
                begin
                suma:=suma+1;
                if (((kx=0) AND (ky=0)) AND (NOT(center))) then
                  begin
                  suma:=suma-1;
                  end;
                if ((NOT(kx*ky=0)) AND (NOT(diagonal))) then
                  begin
                  suma:=suma-1;
                  end;
                end;
              end;
            end;
          for l:=0 to 9 do
            begin
            if ((normes[l]) AND (suma=l)) then buffer[j,k]:=True;
            end;
          NOUVERS[j,k]:=(buffer[j,k] XOR PRIMIVERS[j,k]);
          end;
        if (j mod p1)=0 then
          begin
          StepBar;
          end;
        end;
      PRIMIVERS:=UNIVERS;
      UNIVERS:=NOUVERS;
      end;


Thank you for your kindness, I'm sure this will surely be of help ^^

PS: Sorry for the late reply, I've had a busy summer ^^;; No wait, now I see... possible you wrote it today? amazing, I just checked for the first time in about two months...