Is This Implementation of AnonymousThread Correct?

Question

The MCVE describing the idea is the following. There is an array of classes/records that have a method that takes an input parameter and calculates some result that is stored in one of its variables:

type
  TMyRecord = record
    FLastResult: double;
    procedure DoHeavyMath(Input: double);
  end;
var
  MyRecordArray:array [0..999] of TMyRecord;
  InputData:array [0..999] of double;
implementation

procedure TMyRecord.DoHeavyMath(Input: double);
begin
  FLastResult:=Input;//Here should be some math
end;

I am creating anonymous threads and limit the number of processes (threads) working simultaneously.

procedure TForm1.Button1Click(Sender: TObject);
var
  ThreadsCounter, i: word;
begin
  ThreadsCounter := 0;

  for i := 0 to 999 do
  begin
    while ThreadsCounter >= 4 { or other limit } do
    begin
    end;

    inc(ThreadsCounter);
    TThread.CreateAnonymousThread(
      procedure
      begin
        MyRecordArray[i].DoHeavyMath(InputData[i]);
        dec(ThreadsCounter)
      end).Start;
  end;
  repeat
  until ThreadsCounter = 0;
  // return to main thread tasks
end;

Is this threads' implementation correct?

I'm voting to close this question as off-topic because assuming that you tested your code, and you think it is "correct"; then you are actually asking for a code review; and that should go to codereview.stackexchange.com — GhostCat, Nov 07 '16 at 10:08

score 2 · Accepted Answer · answered Nov 07 '16 at 07:24

2

No it is not correct. There is a data race on ThreadsCounter. Use an atomic function instead, for instance AtomicDecrement.

The performance won't be very good because you incur significant overhead when creating a thread. Use a thread pool instead so that threads can be re-used. The busy loops aren't going to help performance one bit.

answered Nov 07 '16 at 07:24

David Heffernan

601,492
42
1,072
1,490

I've undeleted the Q. SO encourages deleting bad Qs. I can remind you that there are even bages for deleting own Qs having low rating. – asd-tm Nov 07 '16 at 08:17
Changing the `inc` and `dec` to `Atomic[Increment|Decrement]` causes External exception C000001D. – asd-tm Nov 07 '16 at 08:23
Doesn't sound right. Thanks should work. Works for me just fine. Perhaps there's an issue with variable capture in your delphi compiler. Is InterlockedIncrement any better. – David Heffernan Nov 07 '16 at 08:50
`Interlocked[Increment|Decrement]` do not cause exception but deadly hang at `repeat until` section. It seems that the `ThreadsCounter` must be compared to 0 atomicaly – asd-tm Nov 07 '16 at 09:09
No, there's no problem with that. Unless of course your variable is not aligned. I wonder if it is after capture. – David Heffernan Nov 07 '16 at 09:17
It was my fault. I changed the code at the begining of proceudre and the conter was set to 1. We must just use `InterlockedIncrement`. – asd-tm Nov 07 '16 at 09:17
The whole idea to create threads in circle seems to be pernicious. However I have no idea of how to cath the end of calculation of result in each of the pre-created threads without a circle and `WaitFor`. I guess that messages dispatchig is even worse. – asd-tm Nov 07 '16 at 09:22
When an anonymous thread is ending, make a synchroniced call to an event handler. If thread count is zero, you know that all work is done. That will avoid busy loops. – LU RD Nov 07 '16 at 09:28
@LURD Actually it sounds the same as dispatching messages to the Form. OK. For instance the pre-created threads # 1, 2, 3, 4 are started within the for `i:=0 to 999 do` loop. How can we know within this loop that one of the threads (ex. #2) has finished calculations and we can load a new i-th value into it? I see only one way - to create an inner loop and check some flags in the threads. Or the other idea. There should be a secondary TThread starting the tertialy calculating threads and cheking their status in a loop. – asd-tm Nov 07 '16 at 09:43
If you mean to use a thread pool, control the loading from a thread. Use semaphores to signal when a thread is ready for a new job. – LU RD Nov 07 '16 at 10:09

Is This Implementation of AnonymousThread Correct?

1 Answers1