Problem with recursion check in AudioProcessorGraph

There’s a nasty problem with Juce 5.4 related to the recursion test implemented in audioProcessorGraph. I don’t think anyone tried that test with a large number of nodes in the graph - the time to check that graph grows exponentially and becomes unusable if you have more and 15 or so nodes.

You can check out previous threads:

Rail

I’m confused – those threads are from 2014. The issue that I’m talking about is just since Juce 5.4

Perhaps a regression happened and the old exponential time behavior is back again but maybe not because of the same reason…(If I recall right, the old issue was about finding the order in which to reuse audio buffers in the graph.)

As Ralph Waldo Emerson so succinctly put it, “A difference that makes no difference is no difference”

Whatever the reason, that class is unusable right now for any significantly sized graph. We had to revert back to the last version. I hope this will get addressed soon.

What was your “last version” – can you isolate the commit which caused the change for you… I think there hasn’t been a change on the develop branch to this code for a while.

Rail

We were using 5.2 — since we sell a commercial product (and yes, we have a closed-source license), we’re not in a position to just get every change as soon as it comes out – testing is required etc.
Previous to that we were using some 4.x version

Switching from 5.2 to 5.4 was way harder than switching from 4.x to 5.2 — some serious refactoring seems to have been done – and we’re seeing some serious bugs in the latest update.
(For example, opening the AU version of NI’s FM8 shows up in a window that’s 50% larger than the actual plugin)

The threads I posted go back to 2014… but some of them are from 2017 and were related to changes to the class on the develop branch since 4.x

I have a local modified copy of the class which bypasses the RenderSequenceDouble code in buildRenderingSequence since we only use floats and that change doubled the rendering time from 4.x

Rail

Well, I don’t know what to say - but from running in Xcode, we quickly discovered that execution is getting stuck inside this FOR loop for ages - becomes totally unusable.

if (recursionCheck > 0)
    for (auto&& i : dst.inputs)
        if (isAnInputTo (src, *i.otherNode, recursionCheck - 1))
            return true;

I have a reference to the profiling of that method in this post…

Are you running a Debug or Release build… that’s a recursive method so it’ll be extremely slow in a Debug build.

Rail

Yes, I was running a debug build (trying to find out why noise was being produced) when I ran into this issue. I realize debug build is slower than release but it looks like it grows O(n squared) where n is number of edges (connections) and so a debug vs. release build isn’t going to have much impact on it.

We just reverted back to an earlier version of the AudioProcessorGraph

Yes this is a problem with bigger graph, the time spent on isAnInputTo was ridiculous.

Here’s my fix (did not have time to test this, but similar implementation works):

juce_AudioProcessorGraph.h:

typedef HashMap<NodeID,bool> NodeIDBooleanMap;

bool isAnInputTo (Node& src, Node& dst, int recursionCheck, NodeIDBooleanMap &visited) const noexcept;

juce_AudioProcessorGraph.cpp:

bool AudioProcessorGraph::isAnInputTo (Node& src, Node& dst) const noexcept
{
    jassert (nodes.contains (&src));
    jassert (nodes.contains (&dst));
    
    NodeIDBooleanMap visited;

    return isAnInputTo (src, dst, nodes.size(), visited);
}

bool AudioProcessorGraph::isAnInputTo (Node& src, Node& dst, int recursionCheck, NodeIDBooleanMap &visited) const noexcept
{
    if (visited.contains(dst.nodeID))
        return visited[dst.nodeID];
    
    for (auto&& i : dst.inputs)
        if (i.otherNode == &src)
            return true;

    if (recursionCheck > 0)
        for (auto&& i : dst.inputs)
            if (isAnInputTo (src, *i.otherNode, recursionCheck - 1, visited))
                return true;

    visited.set(dst.nodeID, false);
    
    return false;
}