dominators: Implement the Lengauer-Tarjan algorithm

[fitzgen]

An implementation of the algorithm from "A Fast Algorithm for Finding Dominators in a Flowgraph" by Thomas Lengauer and Robert E. Tarjan.

This algorithm runs in O(|E| log |V|) time. This is a better theoretical running time than the "Simple, Fast" algorithm by Cooper (see simple_fast), but has higher start-up costs and memory requirements. The "Simple, Fast" paper reported the tipping point where the Lengauer-Tarjan algorithm outperformed theirs at CFGs with >= 30,000 vertices. Take this with a grain of salt, however, as noted in "Finding Dominators in Practice" by Loukas Georgiadis, Robert E. Tarjan, and Renato F. Werneck:

Even on small instances, however, we did not observe the clear superiority of the ["Simple, Fast" algorithm] reported by Cooper et al., which we attribute to their inefficient implementation of [the simple Lengauer-Tarjan algorithm].

Note that this is an implementation of the simple version of the Lengauer-Tarjan algorithm. There is another version that has an even better theoretical running time of O(|E| α(|E|, |V|)) where α is a functional inverse of Ackermann's function. However, "Finding Dominators in Practice" found it to be slower than the simpler version for all graphs they tested:

Both versions of the Lengauer-Tarjan algorithm (LT [the advanced version]) and (SLT [the simple version]) are more robust [than the "Simple, Engineered" algorithm] on application graphs, and the advantage increases with graph size or graph complexity. Among these three, LT was the slowest, in contrast with the results reported by Lengauer and Tarjan [30]. SLT and [yet another dominators algorithm] were the most consistently fast algorithms in practice; since the former is less sensitive to pathological instances, we think it should be preferred where performance guarantees are important.

---

r? @bluss

cc @m4b

[fitzgen]

Looks like rust 1.12 didn't have closure-that-doesn't-capture-to-fn-ptr yet:

error[E0308]: mismatched types
--> tests/graph.rs:1810:21
|
1810 | test_dominators(|g, r| dominators::simple_fast(g, r));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found closure
|
= note: expected type `fn(&petgraph::Graph<&'static str, ()>, petgraph::prelude::NodeIndex) -> petgraph::algo::dominators::Dominators<petgraph::prelude::NodeIndex>`
= note: found type `[closure@tests/graph.rs:1810:21: 1810:57]`

[fitzgen]

(And the reason this isn't just passing the dominators:: functions directly to test_dominators is because type inference was failing when I did that).

[m4b]

I won't be able to review this in depth, but:

1. documentation and inline comments are totally awesome ❤️
2. I'm excited to compare this against the other implementation; iiuc, this should literally be swappable with simple_fast without any other changes, yes?

[fitzgen]

I'm excited to compare this against the other implementation; iiuc, this should literally be swappable with simple_fast without any other changes, yes?

Yep! They have identical signatures.

[bluss]

Hi -- I haven't had the time to come back to this yet. Feel free to ping me later.

[bluss]

Like #178, I don't really have the bandwidth to review. But it would be cool if contributors reviewed each other's work.

[fitzgen]

Like #178, I don't really have the bandwidth to review. But it would be cool if contributors reviewed each other's work.

Maybe make a dedicated issue seeking more co-maintainers and cc folks who have contributed in the past? Niko recently did that with larlpop and tons of people came out of the woodwork, who were apparently just waiting for someone to reach out for their help.

Personally, I'd also be happy to help.

Since petgraph is the canonical graph crate for Rust, it would be a shame if it languishes or becomes unmaintained.

[khuey]

Since petgraph is the canonical graph crate for Rust, it would be a shame if it languishes or becomes unmaintained.

Word.

[bluss]

Yes, petgraph needs a team. It is only at 0.4.x and has much to do.

• In some way, it is a library around Graph and StableGraph, which are specific graph data structure implementations. In my mind, this is the important part of petgraph.
• With generic associated types later, we can reform all the graph traits into their final and much more useful form
• Implement a better GraphMap (more similar to Graph?)
• Potentially split algorithms from data structures?
• Potentially split graph traits from the data structures?

[fitzgen]

TODO note for myself:

• add a quickcheck to this PR

[waywardmonkeys]

@fitzgen Would you like to revisit this and we can work on getting it landed for 0.5?

[XVilka]

I second this, it is very common algorithm and would be nice to have it out of the box. But it needs more tests than provided. And having quickcheck would help too.

[fitzgen]

I second this, it is very common algorithm and would be nice to have it out of the box.

FWIW, we also have the simple-fast algorithm already in petgraph, so it isn't like there is a giant missing whole IMHO.

@fitzgen Would you like to revisit this and we can work on getting it landed for 0.5?

Unfortunately, I don't think I have the time right now, despite how much I wish that I did :(

[fitzgen]

Unfortunately, I don't think I have the time right now, despite how much I wish that I did :(

Anyone who really wants this algorithm in petgraph: please take over this PR! You have my blessing :)