Reduce allocation overhead in dag_algo::longest_path

[mtreinish]

This commit reduces the overhead from allocations in the longest_path function. Previously for each node being iterated over we were allocating a new vec and growing it as we pushed. This results in a lot of allocations and frees as we iterate over all the nodes. This commit address this overhead by moving to a single vec outside the loop and just clearing it on every iteration. This will reuse the allocation and minimize the overhead this causes.

[jakelishman]

I don't even think you need a Vec at all, no?

Something like

for node in nodes {
    let max_path = graph
        .edges_directed(node, Incoming)
        .try_fold((T::zero(), node), |((max_length, prev), edge)| -> Result<_, E> {
            let source = edge.source();
            let length = dist[&source].0 + weight_fn(edge)?;
            if length > max_length {
                Ok((length, source))
            else {
                Ok((max_length, prev))
            }
        })?;
    // ...

[jakelishman]

Should also be noted that, at least on Qiskit, we don't actually care what the longest path is, just what its cost is, so when we call this function, we're wasting time by backtracking through the hashmap and allocating a path just to throw it away again.

[mtreinish]

Should also be noted that, at least on Qiskit, we don't actually care what the longest path is, just what its cost is, so when we call this function, we're wasting time by backtracking through the hashmap and allocating a path just to throw it away again.

It would be simple enough to add a longest_path_length function to return just the length and skip the unnecessary overhead for Qiskit's use case.

[jakelishman]

btw I'm fine with the change as-is, whether or not you want to refactor to avoid ever having a Vec