Strange problem about QtConcurrent::map

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
_Nemo
Posts: 117
Joined: Sat Feb 06, 2021 8:25 am

Strange problem about QtConcurrent::map

Post by _Nemo »

I have been trying to parallelize recompute recently.
I made the following simple code changes:

Code: Select all

int Document::recompute(const std::vector<App::DocumentObject*> &objs, bool force, bool *hasError, int options)
{
    ...
            for (; idx < topoSortedObjects.size(); ++idx) {
                auto obj = topoSortedObjects[idx];
		...
                if (obj->mustRecompute()) {
                    doRecompute = true;
                    ++objectCount;
                    QList<DocumentObject*> objList;
                    objList << obj;
                    
                    QFuture<int> ans = QtConcurrent::mapped(objList, boost::bind(&App::Document::_recomputeFeature, _1));
                    int res = ans.result();
                    //ans.waitForFinished();
                    
                    /*int res = _recomputeFeature(obj);*/
                    ...
                    }
        ...
	}                    
    ...               
}
However, VS indicates that there is no proper user-defined conversion from Qfuture<void> to Qfuture<int>.
But _recomputeFeature returns int.

Code: Select all

int Document::_recomputeFeature(DocumentObject* Feat)
{...}
I don't know what the problem is. :oops:
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Strange problem about QtConcurrent::map

Post by wmayer »

However, VS indicates that there is no proper user-defined conversion from Qfuture<void> to Qfuture<int>.
This isn't the problem. It's the invalid boost::bind because the function is not bound to a certain Document instance. Btw, instead of boost::bind you should use std::bind.

Code: Select all

namespace sp = std::placeholders;

...
QFuture<int> ans = QtConcurrent::mapped(objList, std::bind(&App::Document::_recomputeFeature, this, sp::_1));
_Nemo
Posts: 117
Joined: Sat Feb 06, 2021 8:25 am

Re: Strange problem about QtConcurrent::map

Post by _Nemo »

wmayer wrote: Thu Feb 09, 2023 12:43 pm ...

Code: Select all

namespace sp = std::placeholders;

...
QFuture<int> ans = QtConcurrent::mapped(objList, std::bind(&App::Document::_recomputeFeature, this, sp::_1));
Thank you very much! The code runs successfully!
But like the QtConcurrent::run I tried before, I ran into a deadlock problem.
How should I break the shackles of GIL? :(
BFYME3V8KKN%N(B1F8[(4BI.png
BFYME3V8KKN%N(B1F8[(4BI.png (74.29 KiB) Viewed 792 times
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Strange problem about QtConcurrent::map

Post by wmayer »

How should I break the shackles of GIL?
When starting the interpreter in FreeCAD (see InterpreterSingleton::init) then the GIL is released immediately using PyEval_SaveThread.
Then every time when executing Python code the GIL is locked temporarily using the class PyGILStateLocker.

Now when the function Document::recompute() is called it's probably called by its Python wrapper DocumentPy::recompute() and at that time the GIL is also locked. In order to avoid a deadlock you must temporarily release the GIL, run the _recomputeFeature and lock the GIL again. Therefore try this inside Document::recompute:

Code: Select all

// add this extra scope to release and lock the GIL automatically
{
    Base::PyGILStateRelease unlockGIL;
    
    ...
    QFuture<int> ans = QtConcurrent::mapped(objList, std::bind(&App::Document::_recomputeFeature, this, sp::_1));
    ...
}
Some remarks:
When a feature is recomputed it notifies its view provider which may change a Qt widget. But changing widgets from within a thread is not allowed and usually leads to a crash.
I wonder how often does it happen that the topological sort gives two or more independent sub-graphs. In most cases if one feature is marked for recompute the topological sort gives you only one sub-graph of features that must be recomputed.
In case a topological sort gives you two sub-graphs it can happen that they cannot be processed in parallel because of the GIL.
_Nemo
Posts: 117
Joined: Sat Feb 06, 2021 8:25 am

Re: Strange problem about QtConcurrent::map

Post by _Nemo »

Code: Select all

// add this extra scope to release and lock the GIL automatically
{
    Base::PyGILStateRelease unlockGIL;
    
    ...
    QFuture<int> ans = QtConcurrent::mapped(objList, std::bind(&App::Document::_recomputeFeature, this, sp::_1));
    ...
}
Thank you so much! The code worked!
Judging from the thread id I output, _recomputeFeature seems to be running in different threads:

Code: Select all

09:20:32  9.7484 <App> Document.cpp(3418): Thread id of recompute() is 00000000000033B4
09:20:32  9.74992 <App> Document.cpp(3766): Thread id of _recomputeFeature() is 0000000000004B10
09:20:32  9.75001 <App> Document.cpp(3767): The feature is test_______#dim
09:20:32  9.75081 <App> Document.cpp(3766): Thread id of _recomputeFeature() is 0000000000003030
09:20:32  9.75088 <App> Document.cpp(3767): The feature is test_______#XZ_Plane
09:20:32  9.75202 <App> Document.cpp(3766): Thread id of _recomputeFeature() is 0000000000003298
09:20:32  9.75208 <App> Document.cpp(3767): The feature is test_______#Sketch002
...
But unfortunately, the actual performance has not improved. In FreeCAD's report browser, this prompt appears:

Code: Select all

09:20:32  QObject::killTimer: Timers cannot be stopped from another thread
09:20:32  QObject::startTimer: Timers cannot be started from another thread
09:20:32  QObject::killTimer: Timers cannot be stopped from another thread
09:20:32  QObject::startTimer: Timers cannot be started from another thread
...
I wonder if this is the problem you mentioned:
When a feature is recomputed it notifies its view provider which may change a Qt widget. But changing widgets from within a thread is not allowed and usually leads to a crash.
...
In this regard, my initial idea is to process the routes in the dependency graph in parallel that can be reccomputed independently.
For example, in the figure below, [1][2], [3][4][5], [6][7][8] can be processed in parallel in theory.
K%)1PSB_53%F499AVO~TRAI.png
K%)1PSB_53%F499AVO~TRAI.png (156.47 KiB) Viewed 693 times
Do you think it is feasible?
Last edited by _Nemo on Sun Feb 12, 2023 2:36 pm, edited 1 time in total.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Strange problem about QtConcurrent::map

Post by wmayer »

Something is wrong with you quoting. Can you fix it, please?
_Nemo
Posts: 117
Joined: Sat Feb 06, 2021 8:25 am

Re: Strange problem about QtConcurrent::map

Post by _Nemo »

wmayer wrote: Sun Feb 12, 2023 9:20 am Something is wrong with you quoting. Can you fix it, please?
What's wrong? My side shows normal.
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Strange problem about QtConcurrent::map

Post by wmayer »

_Nemo wrote: Sun Feb 12, 2023 2:00 pm What's wrong? My side shows normal.
Your whole posting was a single quoting.
_Nemo
Posts: 117
Joined: Sat Feb 06, 2021 8:25 am

Re: Strange problem about QtConcurrent::map

Post by _Nemo »

wmayer wrote: Sun Feb 12, 2023 2:01 pm Your whole posting was a single quoting.
And now?
wmayer
Founder
Posts: 20243
Joined: Thu Feb 19, 2009 10:32 am
Contact:

Re: Strange problem about QtConcurrent::map

Post by wmayer »

Now it's fixed.
Post Reply