views:

610

answers:

9
+4  A: 

I would see if you can do it in one loop as per comment. Also try doing it in a thread so you can eliminate the Application.ProcessMessages and Sleep calls without blocking the UI.

Craig
+1 for the suggestion of a thread for a long-during task that keeps the GUI responsive.
Smasher
+1 for threading.
Jeroen Pluimers
+12  A: 

There are a couple obvious problems with your code.

First off, you're wasting a lot of CPU cycles computing the same values over and over again. The AA..ZZ values aren't going to change, so there's no need to build them over and over. Try something like this: Create a third TStringList. Go through and fill it with all possible AA..ZZ permutations with your double loop. Once that's over with, loop through and merge this list of precomputed strings with the values in slist1. You should see a pretty big boost from that.

(Or, if time is absolutely at a premium, write a minor little program that will compute the permutation list and save it to a textfile, then compile that into your app as a string resource which you can load at runtime.)

Second, and this is probably what's killing you, you shouldn't have the ProcessMessages and the Sleep calls in the innermost loop. Sleep(1); sounds like it means "sleep for 1 milisecond", but Windows doesn't offer that sort of precision. What you end up getting is "sleep for at least 1 milisecond". It releases the CPU until Windows gets back around to it, which is usually somewhere on the order of 16 miliseconds. So you're adding a delay of 16 msec (plus as long as ProcessMessages takes) into a very tight loop that probably takes only a few microseconds to execute the rest of its code.

If you need something like that to keep the UI responsive, it should be in the outermost loop, not an inner one, and you probably don't even need to run it every iteration. Try something like if ch mod 100 = 0 then //sleep and process messages here. Craig's suggestion to move this task to a worker thread would also help, but only if you know enough about threads to get it right. They can be tricky.

Mason Wheeler
Thanks for the information, no I don't know enough about Threads to get it right.. :P I'm a novice at best at programming in Delphi. Do you have any suggestions on where I can find more information on saving it as a string resource?
Brad
Since you're a novice don't bother with putting it in a resource--especially as that likely isn't any faster anyway. Anything you save in not calculating it you probably give back in I/O loading it.
Loren Pechtel
Unless I'm mistaken, `ch mod 100 = 0` will only be true once! `ch mod 10 = 0` might be better..
Blorgbeard
+11  A: 

You should surround your code with slist2.BeginUpdate() and slist2.EndUpdate(), to stop TStringList from doing extra processing.

From my experience, you would get a very large improvement by using fewer ProcessMessages(); Sleep(1); statements, as suggested in other answers.

Try moving it to just below the first for loop, and see what improvement you get.

Blorgbeard
+1 Short and precise. Only the possibility to run it in a thread is missing IMHO. Maybe you can edit it.
Smasher
+2  A: 

OK. I have tried to optimize your code. For final tests, some test-data is needed.

What I have done (it include most of the ideas from Mason):

  • comment out the code about "cancel" and "
  • gave types and variables a more meaningfull name
  • used the names that Delphi uses ("Application" in stead of "application", etc) to make it readable
  • moved some logic into "KeepUIGoing"
  • move the calculation of the suffixes out of the main loop into an initialization loop
  • made it optionally use a TStringBuilder (which can be way faster than a TStringList, and is available since Delphi 2009)

Below is the modified code, let me know if it works for you.

procedure TForm2.Button1Click(Sender: TObject);
{$define UseStringBuilder}
  procedure KeepUIGoing(SourceListIndex: Integer);
  begin
    if SourceListIndex mod 100 = 0 then
    begin
      Application.ProcessMessages;
      // so it doesn't freeze the application in long loops.  Not 100% sure where this should be placed, if at all.
      Sleep(1);
    end;
  end;
const
  First = 'a';
  Last = 'z';
type
  TRange = First .. Last;
  TSuffixes = array [TRange, TRange] of string;
var
  OuterIndex, InnerIndex: Char;
  SourceListIndex: Integer;
  SourceList, TargetList: TStrings;
  Suffixes: TSuffixes;
  NewLine: string;
{$ifdef UseStringBuilder}
  TargetStringBuilder: TStringBuilder; // could be way faster than TStringList
{$endif UseStringBuilder}
begin
  for OuterIndex := First to Last do
    for InnerIndex := First to Last do
      Suffixes[OuterIndex, InnerIndex] := OuterIndex + InnerIndex;

  SourceList := TStringList.Create;
  TargetList := TStringList.Create;
{$ifdef UseStringBuilder}
  TargetStringBuilder := TStringBuilder.Create();
{$endif UseStringBuilder}
  try
    SourceList.Text := queuebox.Items.Text;
    for OuterIndex := First to Last do
    begin
      for InnerIndex := First to Last do
      begin
        for SourceListIndex := 0 to SourceList.Count - 1 do
        begin
          KeepUIGoing(SourceListIndex);
          // if cancel then
          // Break;
          NewLine := SourceList.Strings[SourceListIndex] + Suffixes[OuterIndex, InnerIndex];
{$ifdef UseStringBuilder}
          TargetStringBuilder.AppendLine(NewLine);
{$else}
          TargetList.Add(NewLine);
{$endif UseStringBuilder}
        end;
      end;
    end;
{$ifdef UseStringBuilder}
    TargetList.Text := TargetStringBuilder.ToString();
{$endif UseStringBuilder}
    // insertsingle(TargetList, queuebox);
  finally
{$ifdef UseStringBuilder}
    FreeAndNil(TargetStringBuilder);
{$endif UseStringBuilder}
    FreeAndNil(SourceList);
    FreeAndNil(TargetList);
  end;
end;

--jeroen

Jeroen Pluimers
Thanks for the comments, and the code example... I'm going to try it out in the morning... I didn't know about TStringBuilder, I just started using TStringList and feeling like I'm not going to break something.. :P
Brad
+1  A: 

hi!

try this sample code - hope this will help a little (Delphi 5 Ent./WinXP)

procedure TForm1.Button1Click(Sender: TObject);
var
   i,k: Integer;
   sourceList, destList: TStringList;
   ch1, ch2: char;
begin
   destList := TStringList.Create;
   sourceList := TStringList.Create;

   //some sample data but I guess your list will have 1000+
   //entries?
   sourceList.Add('Element1');
   sourceList.Add('Element2');
   sourceList.Add('Element3');

   try
      i := 0;
      while i < (26*26) do
      begin
         if (i mod 100) = 0 then
            Application.ProcessMessages;

         ch1 := char(65 + (i div 26));
         ch2 := char(65 + (i mod 26));

         for k := 0 to sourceList.Count -1 do
            destList.Add(Format('%s-%s%s', [sourceList.Strings[k], ch1, ch2]));
         Inc(i);
      end;

      Memo1.Lines.AddStrings(destList);
   finally
      FreeAndNil(destList);
      FreeAndNil(sourceList);
   end;
end;    

--Reinhard

pastacool
unless I missed something, this is setup more to add the 676 options to one starting string VS a list of hundreds/thousands.
Brad
yes, you are right, if you add AA to ZZ to a list then there are only 676 possible values (26 * 26). if you need more lets say a third char AAA to ZZZ (26 * 26 * 26) you can handle at most 17576 or do you want to begin with AA again?.
pastacool
I think I understood your problem the wrong way...changed the code
pastacool
+1  A: 

I know this doesn't specifically answer your question, but if you are interested in Delphi algorithm's, Julian Bucknall (CTO of DevExpress) wrote the definitive Delphi algorithms book

Tomes of Delphi: Algorithms and Data Structures:

  • Chapter 1: What is an algorithm?
  • Chapter 2: Arrays
  • Chapter 3: Linked Lists, Stacks, and Queues
  • Chapter 4: Searching
  • Chapter 5: Sorting
  • Chapter 6: Randomized Algorithms
  • Chapter 7: Hashing and Hash Tables
  • Chapter 8: Binary Trees
  • Chapter 9: Priority Queues and Heapsort
  • Chapter 10: State Machines and Regular Expressions
  • Chapter 11: Data Compression
  • Chapter 12: Advanced Topics

You can also get his EZDSL (Easy Data Structures Library) for Delphi 2009 and Delphi 6-2007.

Mick
+1 for mentioning this excellent book!
Jeroen Pluimers
+2  A: 

Below an example of how you might use a secundary thread to do the heavy work.

For the 35 items you mention it is not worth it to start another thread. For a few thousand, the game changes. To process 10000 items takes 10sec on my computer.

some benefits of multithreading include that:

  • the main thread stays responsive.
  • the calculation can be stopped at will.

some pitfalls are that

  • care must be taken (in its current implementation) to not mess with the passed through stringlists while the seeding is running.
  • added complexity of multithreading .

Paste in our favorite editor and you should be able to get it running in no time.

procedure TForm1.btnStartClick(Sender: TObject);
var
  I: Integer;
begin
  //***** Fill the sourcelist
  FSource := TStringList.Create;
  FDestination := TStringList.Create;
  for I := 0 to 9999 do
    FSource.Add(Format('Test%0:d', [I]));

  //***** Create and fire Thread
  FSeeder := TSeeder.Create(FSource, FDestination);
  FSeeder.OnTerminate := DoSeederDone;
  FSeeder.Resume;
end;

procedure TForm1.btnStopClick(Sender: TObject);
begin
  if Assigned(FSeeder) then
    FSeeder.Terminate;
end;

procedure TForm1.DoSeederDone(Sender: TObject);
var
  I, step: Integer;
begin
  I := 0;
  step := 0;
  while I < FDestination.Count do
  begin
    //***** Don't show every item. OutputDebugString is pretty slow.
    OutputDebugString(PChar(FDestination[I]));
    Inc(step);
    Inc(I, step);
  end;
  FSource.Free;
  FDestination.Free;
end;

{ TSeeder }

constructor TSeeder.Create(const source, destination: TStringList);
begin
  //***** Create a suspended, automatically freed Thread object.
  Assert(Assigned(source));
  Assert(Assigned(destination));
  Assert(destination.Count = 0);
  inherited Create(True);
  FreeOnTerminate := True; //***** Triggers the OnTerminate event
  FSource := source;
  FDestination := destination;
end;

procedure TSeeder.Execute;
var
  I, J: Integer;
  AString: string;
begin
  FDestination.BeginUpdate;
  try
    FDestination.Capacity := FSource.Count * 26 * 26;
    for I := 0 to Pred(FSource.Count) do
    begin
      AString := FSource[I];
      for J := 0 to Pred(26 * 26) do
      begin
        FDestination.Add(AString + Char(J div 26 + $41) + Char(J mod 26 + $41));
        if Terminated then Exit;
      end;
    end;
  finally
    FDestination.EndUpdate;
  end;
end;
Lieven
A: 

if you are looking for pure speed just unroll the code into a single loop and write each line as a separate assignment. You could write a program to write the lines for you automatically then copy and past them into your code. This would essentially be about the fastest method possible. Also turn off all updates as mentioned above.

procedure Tmainform.btnSeederClick(Sender: TObject);
var
  ch,ch2:char;
  i:integer;
  slist1, slist2:TStrings;
begin
  slist1:= TStringList.Create;
  slist2:= TStringList.Create;

  slist1.Text :=queuebox.Items.Text;

  slist2.BeginUpdate() 
     for I := 0 to slist1.Count - 1 do
        begin
        application.ProcessMessages; // so it doesn't freeze the application in long loops.  Not 100% sure where this should be placed, if at all.
         if cancel then Break; 
         slist2.Add(slist1.Strings[i]+'AA');
         slist2.Add(slist1.Strings[i]+'AB');
         slist2.Add(slist1.Strings[i]+'AC');
         ...
         slist2.Add(slist1.Strings[i]+'ZZ');
        end;
slist2.EndUpdate()
insertsingle(slist2,queuebox);
freeandnil(slist1);
freeandnil(slist2);
end;
General Jackson Tackett
Oh you could write a little program to create the slist2.add(...'AA') lines so you dont have write them yourself.. :) Hope I understood the problem correctly.
General Jackson Tackett
-1 for a thoroughly unhelpful answer. This features a wrong optimization attempt (unrolling a loop to a sequence of 26 * 26 complex statements) while failing to apply the obvious one (accessing `slist1.Strings[i]` only once). Loop unrolling is definitely counter-productive on modern systems if the resulting code is much larger, due to the lower locality and higher cache invalidation.
mghie
A: 

If you want events to be processed during your loop, such as the Cancel button being clicked, calling Application.ProcessMessages is sufficient. If you call that regularly but not too regularly, e.g. 50 times per second, then your application will remain responsive to the Cancel button without slowing down too much. Application.ProcessMessages returns pretty quickly if there aren't any messages to be processed.

This technique is appropriate for relatively short computations (a few seconds) that you would expect the user to wait on. For long computations a background thread is more appropriate. Then your application can remain fully responsive, particularly if the user has a multi-core CPU.

Calling Sleep in the main thread does not allow your application to process events. It allows other applications to do something. Calling Sleep really puts your application (the calling thread, actually) to sleep for the requested amount of time or the remainder of the thread's time slice, whichever is larger.

Jan Goyvaerts