[m-rev.] for post-commit review: fix a perf regression for --generate-dependencies

Peter Wang novalazy at gmail.com
Fri Aug 15 12:08:29 AEST 2025


On Thu, 14 Aug 2025 11:46:19 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> 
> 
> On Thu, 14 Aug 2025 10:57:23 +1000, Julien Fischer <jfischer at opturion.com> wrote:
> > On Thu, 14 Aug 2025 at 07:24, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
> > > +    %
> > > +:- pred reachable_vertices_from(digraph(T)::in, T::in, set(T)::out) is det.
> > 
> > Some alternative suggestions for names: reachable_vertex_set or
> > reachable_vertices.
> 
> I think reachable_vertices_from is more descriptive than both of those.
> 
> This module has significant naming problems anyway. The main one is
> the use of "vertex", when "node" is more commonly used, shorter,
> and has a regular plural form :-( However, there are others as well, such as
> 
> - lookup_vertex and lookup_key each doing a job that could be better
>   described by the *other* name,
> 
> - lookup_to and lookup_from being worse names than e.g. lookup_successors
>   and lookup_predecessors would be,
> 
> - dfs, and therefore dfsrev, being horribly uninformative names, made worse by ...
> 
> - ... both being used *both* for a local operation starting at a given node, *and*
>   for a global operation on the whole graph,
> 
> - and there are other predicate names that have two different-arity versions,
>   such as "reduced".
> 
> For some of these, I could propose better names, but for the global versions
> of dfs and dfsrev, I can't, because the documentation does not really tell me
> what they are for :-(. I can  read the code, but that does not tell me *why*
> it does what it does. Can anyone reading this email answer that question?

Their documentation seems *fairly* clear: return a list of the nodes in
the graph, such that "parent" nodes appear before "child" nodes,
or the other way around. Unlike return_vertices_in_from_to_order or
return_vertices_in_to_from_order, they don't fail on cyclic graphs.

dfsrev/2 is used in the implementation of cliques/2,
return_vertices_in_from_to_order/2 and return_sccs_in_to_from_order/1.

profiler/propagate.m uses dfsrev/2 in identify_cycles.

I couldn't tell you the difference between cliques/2 and the
return_sccs_* predicates, other than one returns a set,
and the others return a list.

Peter


More information about the reviews mailing list