-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace Lists with Data.Sequence in edge lists. #63
Comments
Is this still the case with #62 merged in? |
Yes, #62 only fixes it if you are adding one edge at a time. If you add n edges at O(1), it gets O(n). Consider the case where you are merging nodes in a Graph (which is very important in my project):
Since the Adj use regular lists, currently, it will be O(n). Using Seq it would be O(log(n)) What I'm now wondering is if it is possible to change the inner workings of fgl to work with something like Monoid or Foldable, or some other type class instead of lists. Anyway, I'll investigate it further in the next week, and provide some evidence if it is something really important. |
It seems #62 fixed my space leaks. |
Ok, I've been working on this problem in my project and my conclusion is that the problem is with my algorithm and lists are more than fine to represent edges. Therefore, this issue should be closed. |
Since it is not really possible to know before hand if
a ++ b
is faster thanb ++ a
, it might be better to use Data.Sequence.This change can improve fgl's
(&)
performance from O(n) to potentially O(1), without loss of generality or ease of use.I noticed this when the following change reduced my memory usage by half:
wuerges/vlsi_verification@45e7c30?diff=unified#diff-014d188201674e3d3062b5a764945832
This is due to fastInsEdge being implemented with
addLists
instead of(++)
I could evaluate this because my particular Graph has a huge amount of inn edges for each node and at most 2 out edges. The change above only made any difference in memory consumption when changing the input edge list, not the output.
The text was updated successfully, but these errors were encountered: