Jump to content
Tommi Prami

ForEach runs only one "thread" at time

Recommended Posts

Code is basically this:

 

        LFiles.AddStrings(TDirectory.GetFiles(LRootFolder, LSearchPattern, TSearchOption.soTopDirectoryOnly));

        if LFiles.Count > 0 then
        begin
          Parallel.ForEach(LFiles).Execute(
            procedure(const AFileName: TOmniValue)
            var
              LCurrentFile: string;
            begin
              LCurrentFile := AFileName;

              CompressFile(LRootFolder, ExtractFileName(LCurrentFile));
            end
          );
        end


LFiles is TStringList;

This is in CommandLine app, so maybe it could not even work this way, and I should use some other coding pattern here than  ForEach. I could not find way to wait the ForEach loop, If I put .NoWait in there.

 

-Tee-

Share this post


Link to post
35 minutes ago, Lars Fosdal said:

No locks or semaphores in CompressFile?

Yes but released in  try.. finally. I use Critical section and very short ones. 

Edited by Tommi Prami

Share this post


Link to post
1 minute ago, Primož Gabrijelčič said:

Looked into that, but did not quite get how it would solve this problem.

Refactored ForEach loop into separate procedure which was used in Future, with same result. Now I let it wun to see if it'll do all "tasks" sequentially.

I was using Delphi PPL and it more than less worked, tried to port it to to OTL, but it was not that simple as I thought. 

Share this post


Link to post

Is CompressFile CPU bound? Are we supposed to know what is in there? Seems like it would be important? 

Edited by David Heffernan

Share this post


Link to post
1 hour ago, Tommi Prami said:
1 hour ago, Lars Fosdal said:

No locks or semaphores in CompressFile?

Yes but released in  try.. finally. I use Critical section and very short ones. 

Doesn't that mean that only ONE compression can happen at a time - i.e. serializing your use of CompressFile?
Edit: Are you reusing the same critical section in multiple locations in the code or on multiple nested levels? That could become a race condition.

 

Share this post


Link to post
13 hours ago, Ondrej Kelle said:

`ProcessMessages` is never called...

There is a loop while waiting the running process that will process messages, but not with ProcessMessages-call but uses PeekMessage with PM_REMOVE flag.

Edited by Tommi Prami

Share this post


Link to post
13 hours ago, David Heffernan said:

Is CompressFile CPU bound? Are we supposed to know what is in there? Seems like it would be important? 

Posted link to the repo later, did not remember that it was public.

Currently it is not CPU bound, uses 10% of CPU and not that much memory also, there is my own CPU and Memory throttling code, but it has no effect. Tested it with putting break point there.,

 

There will be one "thread" processing. With PPL it worked pretty well, with OTL it is running all in sequential manner, not in  parallel, it completed the process but second compression after the first one.

Share this post


Link to post
13 hours ago, Lars Fosdal said:

Doesn't that mean that only ONE compression can happen at a time - i.e. serializing your use of CompressFile?
Edit: Are you reusing the same critical section in multiple locations in the code or on multiple nested levels? That could become a race condition.

 

As far as I know, you can re-enter to critical section, as long as you exit them all.

Was not problem with the PPL anyhow. I can remove Critical section code and test tough...

 

Edit: Removed all critical section calls, does not change the behavior. Also removing my own CPU and Memory Throttling code at WaitForSystemStatus don't change the behaviour either.

Edited by Tommi Prami
Additional info

Share this post


Link to post
3 minutes ago, Primož Gabrijelčič said:

Although, hmmmm, I would expect Parallel.ForEach to work in blocking mode without any message processing. I will look into that as soon as possible.

Take your time.

Share this post


Link to post
5 minutes ago, Primož Gabrijelčič said:

Messages are still not processed while parallel for is executing.

ExecuteAndWait will pump messages, whic is 99.999% of work app does... Waiths 7-zip..

 

-Tee-

Share this post


Link to post
35 minutes ago, Tommi Prami said:

ExecuteAndWait will pump messages, whic is 99.999% of work app does... Waiths 7-zip..

 

-Tee-

ExecuteAndWait processes messages in the worker thread. You have to process messages in the thread that owns the parallel task (in this case the main thread).

  • Like 1
  • Thanks 1

Share this post


Link to post
8 minutes ago, Primož Gabrijelčič said:

ExecuteAndWait processes messages in the worker thread. You have to process messages in the thread that owns the parallel task (in this case the main thread).

Ah OK, did not realize that difference...

 

Thanks for info...

 

-Tee-

Share this post


Link to post

Tried to add .NoWait to ForEach call, and did infinite loop processing messages after that it'll start more than one "task" parallel.

IOmniParallelLoop does not contain flag that I could wait for, that would be nice... (As I saw in some demo maybe in future, IsDone etc...)

 

-Tee-

Edited by Tommi Prami

Share this post


Link to post

How I can wait for the ForEach to finish, if I use the NoWait-pattern?

Could not figure that out yet.

 

-Tee-

Share this post


Link to post
11 minutes ago, Tommi Prami said:

How I can wait for the ForEach to finish, if I use the NoWait-pattern?

Use it's .OnStop function (async) or .OnStopInvoke (synchronized to the owner thread).

Share this post


Link to post
54 minutes ago, Primož Gabrijelčič said:

Use it's .OnStop function (async) or .OnStopInvoke (synchronized to the owner thread).

Thanks... I'll look into that...

 

That seems too obvious now, hopefully in few minutes also 😄 🙂

 

-Tee-

Share this post


Link to post
On 12/11/2023 at 3:17 PM, David Heffernan said:

Is CompressFile CPU bound? Are we supposed to know what is in there? Seems like it would be important? 

The funny thing here, to my mind, is that I'd have expected that the first thing you did would have been to replace the actual function with a pointless work tasks that span in a loop for a certain period of time. If you then found that the behaviour was the same, i.e. not using all processor resources, you would know that the issue was in the library, or how you were using it. In fact had that been the case you could have come up with a very short example to capture the behaviour. You'd probably then have had an answer in an hour or two.

Share this post


Link to post

Thank you all,

 

Now I've ported the code from PPL to OTL, and next I'll polish it a bit and just use it, or maybe tune it a bit,. As is, it's good enough what it does...

 

-Tee-

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×