optimized connected component computation #569
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #569 +/- ##
==========================================
+ Coverage 91.13% 91.18% +0.04%
==========================================
Files 35 35
Lines 4547 4570 +23
==========================================
+ Hits 4144 4167 +23
Misses 403 403
☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| mergeToVert.clear(); | ||
| mergeFromVert.clear(); | ||
| for (int v = 0; v < numVert; ++v) { | ||
| const int mergeTo = label2vert[vertLabels[v]]; | ||
| const int mergeTo = uf.find(v); |
There was a problem hiding this comment.
Nice! Does this mean we can remove label2vert entirely?
There was a problem hiding this comment.
yes, find always return one representative
| I connectedComponents(std::vector<I>& components) { | ||
| components.resize(parents.size()); | ||
| I lonelyNodes = 0; | ||
| std::unordered_map<I, I> toLabel; |
There was a problem hiding this comment.
I can't remember if this is true everywhere, but I recall being annoyed that connected components returned arbitrary sequential labels instead of a representative member, so I ended up having to add code to get back to a representative member. It may be possible that the connected components part of this isn't actually necessary and that find may be all we really need.
There was a problem hiding this comment.
I didn't spend much time looking at the code so I only changed the algorithm for MeshGL::Merge that I can immediately recognize.
It will be much faster if you can just work with a representative member instead of requiring a sequential component ID (and don't need the component count). This connectedComponent is a lot slower than the union part. In fact, it probably took around 80% of the time in GetLabel.
* use union find for graph connected components * update readme * remove graphlite dep
From #561 (reply in thread):
I was thinking about this yesterday, and decided to look at graphlite's connected component algorithm. The algorithm in graphlite and boost graph are using hashmap, which is usually slower than union find. So I implemented union find and found about 50% performance improvement for larger graphs, which is pretty nice.
Further checking shows that there are many vertices with no neighbors, which probably correspond to faces with no coplanar neighbors. Optimizing that by not storing their ID in a hashmap provided further performance improvement.
Optimizing can speed up large mesh import significantly. Here are some of the timings in our test cases (I only printed the timing for
GetLabelswhen numNodes > 10000 to avoid showing too many results)this also removes graphlite from our dependencies.