consider dropping Visitor pattern for querying DAG info

[skriss]

We should consider dropping the Visitor pattern that's used to extract information from Contour's internal model ("the DAG"), and instead have the internal model provide methods for getting information out of it. This would result in simpler code and better runtime performance.

The Visitor pattern is useful when you want to separate the algorithm that operates on an object structure from the object structure itself. This is useful for extensibility, i.e. when an object structure is provided as part of a library, or when the object structure itself changes rapidly or is not known ahead of time. These are not the case in Contour - the user of the DAG is Contour code, and the DAG structure is well-defined and changes infrequently. So we can easily define methods on the DAG struct that understand the structure of the internal model and simply and efficiently extract it for consumers. IMHO this is more straightforward code that's easier to work with.

On the performance front, using the Visitor pattern ends up being a pretty inefficient way to get information out of the internal model, particularly with large sets of configuration, because the entire DAG is visited to get any subset of information out if it. This is because the Visitor pattern makes no assumptions about the structure of the DAG, but instead assumes that any type (e.g. a Listener) could be located at any place in the object structure. If we directly defined methods for getting e.g. Clusters or VirtualHosts out of the model, those methods would "know" where to find that information, and get it in the most efficient way, rather than needlessly visiting irrelevant parts of the object structure.

Interested in others' thoughts here.

[stevesloka]

The original intent of the graph was to find invalid IngressRoutes HTTPProxy resources that didn't have proper includes, etc, etc, but I agree that we're not actually using that any more and the perf benefit would be awesome!

[xaleeks]

Fully support doing this, tagged it 1.21 because 1.20 looks very busy already.

[skriss]

The PR's up already and just needs reviews, so hopefully we can get it into 1.20, but it's not critical.