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

Improve efficiency of arranging nodes #34

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

alphadito
Copy link
Contributor

@alphadito alphadito commented Nov 15, 2023

As the number of nodes and links increase, some performance issues become apparent in the ~100 to ~1000 range.

The performance particularly suffers when the number of links to a join geometry node increases.
Both tree 1 and tree 2 here have ~1000 nodes and ~1000 links but tree 2 has more links going into a single join geometry node.

@tree
def test(): 
    N,K = 100, 8 # Tree 1
    #N,K = 10, 98 # Tree 2
    mesh = cube().mesh
    for i in range(N): 
        meshes = []     
        for j in range(K):
            meshes.append(cube().mesh)
        combined=join_geometry(geometry=meshes)
        mesh = join_geometry(geometry=[combined,mesh])
    return mesh

Time to Generate Topo Sorted Columns of Nodes

Algorithm: Tree 1 Tree 2
Before 25s 228s
After < 0.1s < 0.1s

The performance enhancement is mostly due to avoiding repetitive use of the 'links' property of the NodeSocket class which is currently being invoked for every node input of each node. This is problematic since the links property iterates through all the links of the nodetree. In the new implementation, direct iteration over the links occurs only once to create a graph structure which is then used to topologically sort the nodes.

Other bottlenecks occur at the >1k nodes range.
For example, both running the update function on a node and setting its location
each take about 1.5 seconds on a node tree with 1k nodes but 20 seconds each with 3k nodes (some very weird super quadratic complexity going on here).

I'm not really sure if node.update() is required though. I've tried commenting it out and everything seemed fine ¯\_(ツ)_/¯

api/arrange.py Outdated Show resolved Hide resolved
Copy link
Owner

@carson-katri carson-katri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvements, thanks!

@carson-katri carson-katri merged commit 6952ec5 into carson-katri:main Nov 17, 2023
1 check failed
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

Successfully merging this pull request may close these issues.

2 participants