boostorg/graph

Redundant search for representative in disjoint sets

mglisse opened this issue · 0 comments

We have a general function to join 2 sets

// Union-Set - union the two sets containing vertex x and y
template < class Element > inline void union_set(Element x, Element y)
{
link(find_set(x), find_set(y));
}

This function finds the representative for each set, then calls the more specialized
// Link - union the two sets represented by vertex x and y
template < class Element > inline void link(Element x, Element y)
{
detail::link_sets(parent, rank, x, y, rep);
}

which requires that we pass the representative of each set. It then calls the worker
inline void link_sets(ParentPA p, RankPA rank, Vertex i, Vertex j,
ComponentRepresentative comp_rep)
{
i = comp_rep(p, i);
j = comp_rep(p, j);

which starts by looking for the representative of each set... (if anything, this could be assert(i == comp_rep(p, i));)

It isn't horrible, the search of the representative only takes one query in parent to check that it is indeed already good, it is probably already in cache and the branch predictor probably learns that this comparison is always true. Still, this defeats the purpose of having 2 different functions.

Note that fixing this has the potential to break code for users who mistakenly used link instead of union_sets, and didn't notice since they are currently equivalent.