mixing c++ and python memory models.

Here's the place for discussion related to CAM/CNC and the development of the Path module.
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
janc.linders
Posts: 52
Joined: Sat Jul 25, 2020 2:36 pm

Re: mixing c++ and python memory models.

Post by janc.linders »

Is there a way I can test the changes and see the impact.
User avatar
sliptonic
Veteran
Posts: 3457
Joined: Tue Oct 25, 2011 10:46 pm
Location: Columbia, Missouri
Contact:

Re: mixing c++ and python memory models.

Post by sliptonic »

You can copy the files from the Pull Request and manually copy them to your machine. They're just python so will automatically recompile when you restart freecad.

I'm only holding on the PR because there are two from the same author and together they have a merge conflict. Waiting to know if author wants to merge the PRs into one or have me merge one or the other first.
janc.linders
Posts: 52
Joined: Sat Jul 25, 2020 2:36 pm

Re: mixing c++ and python memory models.

Post by janc.linders »

Thx, meanwhile I found howto get the latest from git and compile/link myself.
Make is running now, but takes time ;)

Is this an option to test the changes ?
User avatar
freman
Veteran
Posts: 2201
Joined: Tue Nov 27, 2018 10:30 pm

Re: mixing c++ and python memory models.

Post by freman »

wmayer found the bug which was causing huge memory losses but it seems the whole c++/py mix in inherently messy. Maybe some things could be improved if python guys were more aware of the guts of CXX code and how to avoid the pitfalls and limitations ( but that is probably not what python progammers are motivated to do in general, they like the ease of use and may not even follow c++ and intricacies of CXX ) .

The Path.Commands.parameters object is a case in point.

Every time a value is assigned, the old c++ PyOjbect is destroyed, a new on created and content copied over by the getter/setter methods of the c++ code.

Someone happily programming python would just expect a increment of the ref counter to the existing python object. A negligible cost operation. Instead it becomes quite a complex operation even if most of it is written in c++.

I still don't have a full understanding of all pseudo python objects in CXX and the implications but if this kind of inefficiency is generalised, at the level of every point, vector, edge , wire etc., and the multiple different representations of basic geometric elements in different libraries and the constant need for converstion classes. this could account for why FreeCAD is so slow as soon as complexity mounts.

The question I'm trying to understand here is whether this is an inherent limitation of trying to mix two very differently structured languages ( my original "bastard mix" ) or whether there are code improvements which could be sought.

Is the getter and setter for the parameters Pdict property unnecessarily cumbersome, could this be done better?
If it does have to be this convoluted, is there any gain in trying to do some of the work in c++ ?

Does anyone have any insight on that ?
User avatar
Kunda1
Veteran
Posts: 13434
Joined: Thu Jan 05, 2017 9:03 pm

Re: mixing c++ and python memory models.

Post by Kunda1 »

freman wrote: Thu Oct 29, 2020 8:57 am wmayer found the bug which was causing huge memory losses but it seems the whole c++/py mix in inherently messy.
Can we close issue #3340 since the issue is more systemic? Maybe open another ticket that is more nuanced and isn't a release blocker since the whole CXX code won't be addressed in the 0.19 cycle...amirite?
Alone you go faster. Together we go farther
Please mark thread [Solved]
Want to contribute back to FC? Checkout:
'good first issues' | Open TODOs and FIXMEs | How to Help FreeCAD | How to report Bugs
etrombly
Posts: 144
Joined: Thu Dec 05, 2019 6:50 pm

Re: mixing c++ and python memory models.

Post by etrombly »

freman wrote: Thu Oct 29, 2020 8:57 am wmayer found the bug which was causing huge memory losses but it seems the whole c++/py mix in inherently messy. Maybe some things could be improved if python guys were more aware of the guts of CXX code and how to avoid the pitfalls and limitations ( but that is probably not what python progammers are motivated to do in general, they like the ease of use and may not even follow c++ and intricacies of CXX ) .

The Path.Commands.parameters object is a case in point.

Every time a value is assigned, the old c++ PyOjbect is destroyed, a new on created and content copied over by the getter/setter methods of the c++ code.

Someone happily programming python would just expect a increment of the ref counter to the existing python object. A negligible cost operation. Instead it becomes quite a complex operation even if most of it is written in c++.

I still don't have a full understanding of all pseudo python objects in CXX and the implications but if this kind of inefficiency is generalised, at the level of every point, vector, edge , wire etc., and the multiple different representations of basic geometric elements in different libraries and the constant need for converstion classes. this could account for why FreeCAD is so slow as soon as complexity mounts.

The question I'm trying to understand here is whether this is an inherent limitation of trying to mix two very differently structured languages ( my original "bastard mix" ) or whether there are code improvements which could be sought.

Is the getter and setter for the parameters Pdict property unnecessarily cumbersome, could this be done better?
If it does have to be this convoluted, is there any gain in trying to do some of the work in c++ ?

Does anyone have any insight on that ?
There isn't any inherent problem with interop between multiple languages. You just need to audit those portions of the code a little more carefully. I think it's just the same issue as many other areas, a lack of developers with the experience to fix those issues.

As mentioned before most of the slow operations that I've personally seen are either from calculations that are slow no matter the language, i.e. converting from brep to mesh (this causes a lot of problems for path), or from logical problems with the code, which is also not necessarily related to what language you're using.

Ultimately I'd just recommend when you encounter a slow operation, profile it to see where the problem lies. Doing more in c++ may be a performance win, but I don't think we have resources for that. Slightly slower, but working code, is better than no code at all.

As a side note, I checked the path codebase for the issue that wmayer found. It does happen in one other place, Path/App/TooltablePyImp.cpp in TooltablePy::getTools.
User avatar
freman
Veteran
Posts: 2201
Joined: Tue Nov 27, 2018 10:30 pm

Re: mixing c++ and python memory models.

Post by freman »

Thanks very much for the input and the extra checking.
Is the getter and setter for the parameters Pdict property unnecessarily cumbersome, could this be done better?
This is one concrete example which was at the heart of the massive post-processor memory hog.

Could you advise on that? If CXX is to be used to provide python extensions like this wouldn't it be best to ensure it follows behaviour that python programmers would expect from the python end without them needing to look into the guts of the python extension to realise it does not work the same as all their python experience would lead them to expect? I can't see why there is this need to create duplicates of data structures via getters and setters instead of simply returning the object and incrementing the ref counter as python would do internally.

I've not tried this kind of hybrid coding of python extensions, maybe I'm missing some crucial point and there is a need to do it this way.
etrombly
Posts: 144
Joined: Thu Dec 05, 2019 6:50 pm

Re: mixing c++ and python memory models.

Post by etrombly »

freman wrote: Mon Dec 14, 2020 3:41 pm Thanks very much for the input and the extra checking.
Is the getter and setter for the parameters Pdict property unnecessarily cumbersome, could this be done better?
This is one concrete example which was at the heart of the massive post-processor memory hog.

Could you advise on that? If CXX is to be used to provide python extensions like this wouldn't it be best to ensure it follows behaviour that python programmers would expect from the python end without them needing to look into the guts of the python extension to realise it does not work the same as all their python experience would lead them to expect? I can't see why there is this need to create duplicates of data structures via getters and setters instead of simply returning the object and incrementing the ref counter as python would do internally.

I've not tried this kind of hybrid coding of python extensions, maybe I'm missing some crucial point and there is a need to do it this way.
Do you mean CommandPy::setParameters in CommandPyImp.cpp? It creates a copy of the input args, which I'm not entirely sure is needed, but it just updates the existing parameter object. Once the input args go out of scope they'll be dropped, so it shouldn't be an issue now that wmayer fixed the ref counter problem.

getParameters does create a new python dict, but that's because internally in c++ that object is a std::map, there is no already existing python object to point to. I could see how this would be a problem if you are calling getParameters from several places and those variables are staying in scope. The only easy fix I see for this is having getParameters take a python list as an argument that it then fills. That would be a little more explicit about what is happening on the c++ side.
User avatar
freman
Veteran
Posts: 2201
Joined: Tue Nov 27, 2018 10:30 pm

Re: mixing c++ and python memory models.

Post by freman »

Thanks, yes that's what I was looking at . More specifically the getter.

It seems grossly inefficient. In the post processors ( which are all basically the same code tweaked a bit ) there is a nested loop which contains about a dozen refs to the variable. Each one calls the getter:

Code: Select all

Py::Dict CommandPy::getParameters(void) const
{
    Py::Dict dict;
    for(std::map<std::string,double>::iterator i = getCommandPtr()->Parameters.begin(); i != getCommandPtr()->Parameters.end(); ++i) {
        dict.setItem(i->first, Py::Float(i->second));
    }
    return dict;
}
so each ref to this object ends up populating a new PDict , copying it all across and returning a new object and then the less than efficient python GC has to clean up the old one. That's probably longer than the rest.

Even now the refcount error which brought attention to this is fixed, I'm not clear why all this copying is necessary. If this also the case with some of the low level stuff shifting back and forth between the multiple representations of geometric elements like vectors, points etc as required by the various libraries in use, there could be slow down which may be significant.

I found that making a local copy in second python variable so that I only accessed the parameters property once got about 25% speed increase in the post proc.
Post Reply