[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:105
+  // Memorizes calculated ranks of head vertices.
+  DenseMap<T, int> Rank;
+};
----------------
mkazantsev wrote:
> mkazantsev wrote:
> > 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.
> We could store a union, though...
After giving it some thought, I don't think it's a good idea. Storing pairs is expensive due to reason I wrote above, storing unions doesn't allow us to understand where we should stop while traversing parents to find head. If we want to do so, we need an extra flag. I'd leave it as is unless we want to complicate this plece of logic without obvious benefits.


https://reviews.llvm.org/D40427