[PATCH] D40427: [ADT] Introduce Disjoint Set Union structure

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: include/llvm/ADT/DisjointSetUnion.h:84
+      // recursively for all chain of parents.
+      return Parent[X] = head(It->second);
+    // Every vertex that doesn't have a parent is the head of its set.
----------------
sanjoy wrote:
> Please avoid recursion here, unless you're certain this would be (say) less than 10 frames for all practical cases (in which case add an assert).
I'm pretty certain that the expected depth is effectively small, but I was also thinking to rewrite this with loop, so I'll do it.


================
Comment at: include/llvm/ADT/DisjointSetUnion.h:105
+  // Memorizes calculated ranks of head vertices.
+  DenseMap<T, int> Rank;
+};
----------------
sanjoy wrote:
> How about just one `DenseMap` that maps `T` instances to a `std::pair<T, int>`?
That would consume as twice as much memory. We only store Rank for roots and Parent for non-roots, and this would have us store both for both.


https://reviews.llvm.org/D40427