Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getSubObjects/getSubObject interface is very slow #43

Closed
abetis opened this issue Sep 13, 2020 · 6 comments
Closed

getSubObjects/getSubObject interface is very slow #43

abetis opened this issue Sep 13, 2020 · 6 comments

Comments

@abetis
Copy link

abetis commented Sep 13, 2020

I'm implementing a feature (in python) for Assembly4 that have to traverse over all the elements in the tree and set Visibility for some of the entries.
I have a not-so-big model, but found that passing all the entries takes lots of time.
Trying to debug and understand why:
Total number of getSubObjects calls was 4189
Total numbers of getSubObject(name, 1) calls was 1909
Total number of time Visibility feature was set was 763 (there are some links to the same part that I will try to recognize and improve the logic).
All that takes about 6.5 seconds on my computer.

I thought the fact that getSubObjects returns only names of the objects and for each there is a need to query the object itself with getSubObject function was a problem, so I've tried to add an API that will do that in C++ code and return the list of the objects with one call. Maybe I did something not that good, but the gain wasn't significant.

Next change was to check the object type before calling the getSubObjects, so the script will handle only App::Part containers.
That lead to the following numbers:
Total number of getSubObjects calls was 289
Total numbers of getSubObject(name, 1) calls was 1909
Total number of time Visibility feature was set was 763.
All that took about 5.5 seconds - a save one 1 second.

That lead me to the obvious conclusion that passing large lists via the API and processing all of them in Python is not efficient, especially when most of the entries are ignored.

How hard will it be to implement a filter in the getSubObjects, by passing a list of object types I would like to get back?
For some calls I need only App::Part, for others I need few types that are used for LCS representation ['PartDesign::CoordinateSystem', 'PartDesign::Plane', 'PartDesign::Line', 'PartDesign::Point'], for some calls I need the body elements...

Maybe there will be needs for other filtering options as well.
Right now for my current implementation I think the TypeId filter will improve the speed drastically.

Can you point me to the needed direction, so I could add such filter and test?
For a start, couldn't find how TypeId property is implemented.

Maybe you have other thoughts in that direction.

Thanks.

@realthunder
Copy link
Owner

There is indeed some overhead in Part's getSubObject(). But shouldn't affect that much. Can you please post here the script you wrote? I'll take a look.

@abetis
Copy link
Author

abetis commented Sep 14, 2020

The overhead is probably not in the APIs, but in the fact that python have to handle/check very large non-needed entries the API is returning.
Since there is no filtering, looks like there is no other way...

Here's a PR:
Zolko-123/FreeCAD_Assembly4#114

Will appreciate your comments if I did something stupid there

@abetis
Copy link
Author

abetis commented Sep 20, 2020

I'm still investigating that direction...
Since I don't know how to add list of string parameter to the functions with all the python/C++ conversion, I decided to abuse the reason parameter for a test.
With some prints I figured out that the GeoFeatureGroupExtension::extensionGetSubObjects function fills the list.
I've tried to add the following code to the function to do the filtering in that function:

bool GeoFeatureGroupExtension::extensionGetSubObjects(std::vector<std::string> &ret, int reason) const {
    Base::Type typeLink = Base::Type::fromName("App::Link");
    Base::Type typePart = Base::Type::fromName("App::Part");
    Base::Type typeBody = Base::Type::fromName("PartDesign::Body");

    Base::Type typeCoordinate = Base::Type::fromName("PartDesign::CoordinateSystem");
    Base::Type typePlane = Base::Type::fromName("PartDesign::Plane");
    Base::Type typeLine = Base::Type::fromName("PartDesign::Line");
    Base::Type typePoint = Base::Type::fromName("PartDesign::Point");

    for(auto obj : Group.getValues()) {
        Base::Type objType = obj->getTypeId();
        if(reason == 8 && (objType != typeLink && objType != typePart && objType != typeBody)) continue;
        if(reason == 9 && (objType != typeCoordinate && objType != typePlane && objType != typeLine && objType != typePoint)) continue;

        if(obj && obj->getNameInDocument() && !obj->testStatus(ObjectStatus::GeoExcluded))
            ret.push_back(std::string(obj->getNameInDocument())+'.');
    }
    return true;
}

Test failed... Not it takes a bit more time than before :(
So filtering on the C++ level doesn't help either...

@realthunder
Copy link
Owner

I thought you've already solved problem by reading your comments in that issue page?

What is the scope of your finding. Do you want to find certain type of objects in the whole document or from some parent object?

@abetis
Copy link
Author

abetis commented Sep 20, 2020

My feature works, but it takes too long to complete as I wrote in the first post of that issue...
Sometime it's from parent object, sometimes its from the top of the document.
Is there any other API besides the getSubObjects/getSubObject couple?

@abetis
Copy link
Author

abetis commented Sep 20, 2020

WOW...
Decided to check another direction...
The same loop code on the sub-objects, but commented out setting the object.Visibility attribute.
The execution time dropped from 3.8sec to 0.02sec!
So that's not the getSubObjects/getSubObject issue, but somewhere in the Visibility attribute implementation.
Will check that direction.
Closing that issue. Thanks.

@abetis abetis closed this as completed Sep 20, 2020
realthunder pushed a commit that referenced this issue May 16, 2022
 [Path] Fix python style for initial message
(now recognized by updatets.py script)
realthunder pushed a commit that referenced this issue May 16, 2022
[Path] Final fix of Path - message during startup  (#43)
@yahbluez yahbluez mentioned this issue Jan 18, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants