Optimize the implementation of Graph.listParents #209
Conversation
| @@ -5,7 +5,14 @@ | |||
| ------------------------------------------------------------------------------} | |||
| {-# language ScopedTypeVariables#-} | |||
|
|
|||
| module Reactive.Banana.Prim.Graph where | |||
| module Reactive.Banana.Prim.Graph | |||
There was a problem hiding this comment.
To make sure some invariants are not violated, made Graph an opaque type to the rest of the codebase.
| @@ -18,8 +25,19 @@ import Data.Maybe | |||
| Graphs and topological sorting | |||
| ------------------------------------------------------------------------------} | |||
| data Graph a = Graph | |||
| { children :: Map.HashMap a [a] | |||
| { -- | The mapping from each node to the set of nodes reachable by an out-edge. If a node has no out-edges, it is | |||
There was a problem hiding this comment.
Just a bit of documentation about how this particular flavor of directed graph representation works.
| @@ -47,9 +65,9 @@ getParents gr x = maybe [] id . Map.lookup x . parents $ gr | |||
| listParents :: forall a. (Eq a, Hashable a) => Graph a -> [a] | |||
| listParents gr = list | |||
| where | |||
| -- all nodes without children | |||
| -- all nodes without parents | |||
There was a problem hiding this comment.
I'm pretty sure this was just a confusing typo, and not a bug in the implementation. We keep nodes that have no parents with this computation:
[ x | x <- Set.toList $ nodes gr, null (getParents gr x)]
|
Hmm... uh, CI looks broken. I guess that's my fault due to #207...? Seems like the line |
|
A previous build seems to have worked. https://github.com/HeinrichApfelmus/reactive-banana/runs/2369884533?check_suite_focus=true#step:12:5 I guess something's gone wrong with the caching. |
mitchellwrosen
force-pushed
the
optimize-list-parents
branch
from
9569314
to
15f200d
Compare
mitchellwrosen
force-pushed
the
optimize-list-parents
branch
from
15f200d
to
25ffe72
Compare
|
Ready for review 👍 |
|
Looks good, thanks! I have added a suggestion. Not sure what happens if I click the "Commit suggestion" button. 🤔 Could you include it? Then feel free to merge. 😊 |
Co-authored-by: Heinrich Apfelmus <apfelmus@quantentunnel.de>
This PR optimizes the implementation of
Graph.listParents. I'll annotate the PR with some comments.