[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