Was a temp test, need to remove

Done

remove

Done

Definitely shouldn't be in loop...

Removed

This part is pure copy-paste from gexecutor.cpp, perhaps it should be somewhere in common (e.g gcommon.hpp)

@dmatveev This is the place where meta is stored into Mag. I assume there might be some data race that's why Mag is currently protected by mutex.

there won't be a race if we statically preallocate the magazine for all relevant [rc.id]s in all relevant types, but the mutex is fine and we can do that later.

Left mutex so far

Removing lock causes the data race even though magazine::getArg is supposed to be a read operation

I assume it's impossible that two Island's write to the same memory in parallel but it caused data race if mutex isn't used

exec() is redundant for IslandActor, in fact GIslandExecutable::Ptr can be stored & accessed in GThreadedExecutor and passed to IslandActor by pointer/reference.

Please, leave a comment in code

In fact it's not necessarily to acquire the mutex on add() method, task_counter might be an atomic.

But since we still need the mutex for wait (as it's uses condvar) atomic + mutex seems the weird combination and didn't give any significant performance improvements. Decided to keep it simple so far.

If fact WaitGroup can be implemented without condvar and mutex by using 2 atomics. But it requires c++20 for this thing: https://en.cppreference.com/w/cpp/atomic/atomic/wait

WaitGroup has been removed

ThreadSanitizer also signals problems when schedule is called from different threads but it seems to be fine since both WaitGroup::add() and Queue::push are supposed to be thread-safe

Empty task - termination criterion for the worker, at least worth a comment...

Done

Yet another copy-paste of this thing

Should be exposed into python later

probably here we should stick to the OpenCV thread settings: https://docs.opencv.org/4.9.0/db/de0/group__core__utils.html#ga2db334ec41d98da3129ef4a2342fc4d4

...but normally it is needed to respond/handle to the setNumThreads() which sets # of threads to OpenCV globally (as it is still a set of standalone functions)

Good point, I think we can use cv::getNumThreads() there instead. Done

is it a third thread pool in this only module? :)

I guess you refer to RequestPool's from ie and ov backends.

normally we have break in the following line

(or the whole case ...: ...; break in a single line)

It's just a copy-paste from gexecutor.cpp but no problem to address it :)

Done

there won't be a race if we statically preallocate the magazine for all relevant [rc.id]s in all relevant types, but the mutex is fine and we can do that later.

I believe it is an abstraction leak to expose 1) a data structure and 2) its protection mutex by a reference to some external context like this.

If the goal of this whole stub is to populate an input vector for an island, you can query the object who owns those two (a magazine and a mutex) to do that for you. Then this thread safety concern falls to that context. So what you could do instead is to have a GraphState structure - representing a running graph - and host the magazine in.

The pro of this way is that later you may have a single executor serving multiple executable graphs - to their state will be clearly separated one from another.

Done

doesn't look good tbh :)

Quite handy though, user doesn't need to keep ThreadPool for scheduling nested tasks.

static void foo() {
    // do something
    ThreadPool::get()->schedule(bar);
};

static void bar() {
     // do something
     ThreadPool::get()->schedule(baz);
}

...
ThreadPool tp;
tp.schedule(foo);

But if we have plan to make ThreadPool available for all GCompiled's, it can be removed for now

Removed

why do you need current at all?

Answered above

Removed

so you increase you wait count once you schedule a task, and decrease once you're done with it -- why?

The entire WaitGroup are used to implement ThreadPool::wait() that is supposed to block until all scheduled tasks, including nested ones, are completed

not relevant, WaitGroup has been removed

Does this mean you terminate your worked threads once the graph is done?

The answer is NO, workers are terminated in the ThreadPool::stop that called during GCompiled destruction.

Not it's done in shutdown method which is private and called only in ~ThreadPool()

Shouldn't this be &&task ?

Done

Can we get a simultaneous read + write situation for the exception? Shouldn't it be protected (e.g. atomic)?

IslandActor itself doesn't give any thread-safe guarantees, besides this method is called only in main thread when all tasks are completed.

Do we care if this line can be unreachable due to hang/exception in task above?

It's assumed that task can't throw exception at this point.

Make sense to check it with something like this: static_assert(noexcept(task()))
Since this is available since C++17 only

Looks it's not memory_order::seq_cst for sure

Done

Not needed any more

Done

Left mutex so far

probably it's not ok to have cv::getNumThreads() called in the header.

Instead, you could introduce two constructors, a default one () with nthreads set to cv::getNumThreads() in its initializer list, and the value-based one.

Can't we just declare it here and move implementation to a cpp file?