Jake Evans
Jake Evans

Reputation: 1048

How to make multiple threads do work on a single TStringList

I want to be able to make multiple threads do work on a single TStringList. This is my current Thread code for using a single thread to do the work;

type
  TFilterThread = class(TThread)
  protected
    procedure Execute; override;
  public
    lCombos : TStringList;
    //Public Vars
  end;

procedure TFilterThread.Execute;
var
  I: integer;
  HTML: String;
  frm1 : TForm1;
  splitLogin: TStringList;
  validCount: Integer;
begin
  validCount := 0;
  for I := 0 to lCombos.Count - 1 do
    begin
      if Terminated then Exit();
      Unit1.Form1.ListBox1.ItemIndex := i;
      try
        HTML := Unit1.Form1.IdHTTP1.Get(EmailCheckURL + lCombos[i]);
        if AnsiPos('You indicated you', HTML) > 0 then
        begin
          //Do stuff
        end;

      except
        Continue;
      end;
    end;
    Showmessage('Finished!');
    Unit1.Form1.Button1.Caption := 'Start';
end;

To Start the thread I use;

lComboLsit := TStringList.Create;
for I := 0 to listBox1.Items.Count -1 do
    lComboLsit.Add(listBox1.Items[i]);`

iTFilterThread := TFilterThread.Create(True);
iTFilterThread.FreeOnTerminate := True;
iTFilterThread.lCombos := lComboLsit;

iTFilterThread.Start;

How would I introduce another thread to also do work on the lCombos list so that the operation would complete quicker?

Upvotes: 1

Views: 1370

Answers (1)

David Heffernan
David Heffernan

Reputation: 612954

The answer is that it depends. If the string list is not modified, then there's nothing to do. If the string list is modified then what is best depends critically on your specific usage.

Looking at your code, your program may not be CPU bound and so adding more threads may not help. Your program's bottleneck will the HTTP communication. However, despite not being CPU bound it is plausible that running multiple threads will reduce the impact HTTP latency. You can benchmark to find out. You don't appear to be modifying the string list so there's no race problems to be concerned with there.

However, here's a problem:

Unit1.Form1.IdHTTP1.Get(EmailCheckURL + lCombos[i]);

That will work, and is thread safe so long as TIdHTTP is thread safe. But it's pretty ugly to allow a thread to access a component on a form like that. And I don't see any real sense or need to share the TIdHTTP instance between threads. It would be far cleaner to let each thread instantiate and use their own TIdHTTP component.

Of course, you will need to decide on a policy for dividing the work between all your threads. You could have a shared index that keeps track of the next item to process. Have each thread increment it atomically each time they take an item. A parallel for loop would be a good fit here. That's available in the latest version of Delphi, or in any decent third party parallel library.

You do have some problems with your code. In the thread procedure you do this:

Unit1.Form1.ListBox1.ItemIndex := i;
....
Unit1.Form1.Button1.Caption := 'Start';

You cannot access VCL components from a thread.

And then you do this

ShowMessage('Finished!');

Don't show UI from a thread.

A point of idiom. Instead of looping over the items in your list box you can simply do this:

lComboLsit.Assign(listBox1.Items);

Upvotes: 3

Related Questions