Help in multithreading design

[Leledumbo]

I'm about to release a new version of one of my softwares which is a GUI frontend for UPX. The new capability is multithreaded compression. Upon releasing, I look into the multithreading code again and not really sure about the design, so I recode that part but now I got confused. Both seems incorrect for me, there must be a better way.

In design 1, the main thread creates other threads and then loop until all items are processed (number of currently finished item is given to each thread, shared and protected inside critical section). Inside the loop, I put Application.ProcessMessages; followed by Sleep(100); to give some time so the main application doesn't hang too much. The main thread also updates a progress bar during (and after) the loop. Other threads update their corresponding line in a TListView (through synchronize method).

In design 2, there's another thread helper that creates other threads, and updates the progress bar (through synchronize method). The execute method of this thread is more or less equal to main thread code in design 1 (i.e. the while loop) but without Application.ProcessMessages or Sleep call because I assume since it's implemented in a thread, it won't hang the main thread. Other threads still do the same, update their corresponding line in a TListView. When all items have been processed, the helper thread enables GUI controls and terminate. In this design, the main thread does nothing but creating the helper thread.

In both design, main thread disables GUI controls before doing any of the above steps.

Which do you think is better? Or if both are wrong, what's your suggestion? I attach both projects if you need to inspect the code. Sorry, I didn't include UPX executable because it's too big to fit as attachment.

[Blaazen]

I am trying the second version.
Where can I get unit "ColorProgress"?

Thanks.

[Leledumbo]

Whoops, sorry! Attached.

[Blaazen]

On 'Run', of some reason, it tells me:

Code: [Select]

No program file
"........................../Forum_Projects/UPX_frontend/ExPress"
found.'Quick compile' works fine but 'Run' gives this message.
I have there Express.lpi, -.lpr -.res -.manifest -.ico
Everything seems normally. I just can't find where is problem.

[Leledumbo]

It seems like the IDE can't find the executable. Could you clarify that it exists and try running it outside the IDE? If it can, perhaps you could try setting different target file name (-o).

[Blaazen]

I got it. It was permission problem. I had good user name but bad user group ('bundle'). It probably happened during decompression.
Anyway. When I select one executable (EDIT: and I press Execute button) it gives error and debugger raises:
Exception: EThread. Semaphore init failed (possibly too many concurrent threads).

It points to the last line:

Code: [Select]

constructor TUPXThreadRunner.Create(AForm: TMainForm; const AUPXFile: String);
begin
  FreeOnTerminate := true;
  FForm := AForm;
  FUPXFile := AUPXFile;
  FThreadCount := 0;
  FNextFileIndex := 0;
  FDoneCount := 0;
  inherited Create(false);  // <--- HERE
end;           

BTW: Button "Help" is not impemented yet.

EDITED.

[Leledumbo]

When I select one executable (EDIT: and I press Execute button) it gives error and debugger raises:
Exception: EThread. Semaphore init failed (possibly too many concurrent threads).

Hmm... I have no idea with that, probably somebody else needs to come here. I haven't tested this new code on *nix though, only Windows.

EDIT: found the cause, I don't have UseCThreads defined, please recompile with that defined.

BTW: Button "Help" is not impemented yet.

It's in "to do" (since first release :p)

[Blaazen]

I am still testing the second version. I made the threads working but I found strange behaviour.
I have dual core CPU.
When I run "upx --best --overlay=copy PATHorFILENAME" from terminal then only one of my cores is loaded to 100%.
When I select one big binary and press "Execute" then both of my cores are loaded to 100%.
I tried to modify the code:

Code: [Select]

procedure TUPXThread.Execute;
var
  TempPos: LongWord;
begin
  with TProcess.Create(nil) do
    try
      try
        Options := [{poWaitOnExit,} poNoConsole, poUsePipes];  //<---- HERE I commented poWaitOnExit
        Priority := FPriority;
        CommandLine := FCmdLine;
        Execute;
        while Running=True do sleep(100);   //<------ and HERE I added waiting loop with sleep()
        if ExitStatus <> 0 then begin   
        ....................................
but no change, still both cores are 100% loaded.
And finally when I removed the waiting loop then it worked only in one core but the GUI was enabled and compression ran in background.
I don't want to say it is bug in FPC - I don't have so much experience with threads. AFAIK there were some changes with Threads recently.
BTW isn't it too complicated? Main program creates thread which creates another thread which creates process? IMO one thread can be removed.

One detail:
Caption of checkbox could be multiline or GroupBox can have some autosize. (See screenshot)

EDIT: The two-core issue happens always, i.e. if I run compiled ExPress from or outside Lazarus has no effect.

[Leledumbo]

And finally when I removed the waiting loop then it worked only in one core but the GUI was enabled and compression ran in background.

My quick guess is that one core executing UPX and the other executes the while loop.

BTW isn't it too complicated? Main program creates thread which creates another thread which creates process? IMO one thread can be removed.

Yup, that's why I asked this. 2nd version (should) give better GUI feeling because the control updates happen in another thread (thus main thread is more responsible, remember I have plan to add cancellation feature), but the many threads itself complicates the process. How about the 1st version?

[Blaazen]

I just tried the first design and it works well. Only one CPU core is loaded.

Maybe compressed files could be automaticly unchecked in listview after "Execute".
Also in the first design is missing {$DEFINE UseCThreads}

[Leledumbo]

I just tried the first design and it works well. Only one CPU core is loaded.

Hmm... so I guess it's better than the 2nd one (though I a bit don't like that Application.ProcessMessages; Sleep(100); pair).

Maybe compressed files could be automaticly unchecked in listview after "Execute".

Maybe... but would someone decompress what he just compress?

Also in the first design is missing {$DEFINE UseCThreads}

Yes, I mainly develop on windows, and linux is just for testing so things like this often forgotten.

[Blaazen]

Maybe... but would someone decompress what he just compress ?

I didn't know that unchecked files in list are decompressed. During testing I always refreshed "big" binaries from backup - to have material for testing.
You should mention it somewhere in IDE (Label or column in ListView). I thought that unchecked items are simply skipped.

[Leledumbo]

I didn't know that unchecked files in list are decompressed. During testing I always refreshed "big" binaries from backup - to have material for testing.
You should mention it somewhere in IDE (Label or column in ListView). I thought that unchecked items are simply skipped.

Hmm... there was a Hint in the ListView, maybe it's deleted accidentally. I'll add that before releasing. Thanks for everything.