Skip to content

Improve performance of graphFromEdges #1151

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

meooow25
Copy link
Contributor

@meooow25 meooow25 commented Jun 27, 2025

  • Replace array with listArray so we don't need to allocate (index, element) pairs.
  • Return -1 from the binary search when the key is not found to avoid Just allocations.

Related: #917


Benchmarks with GHC 9.12.2:

Name                                         Time - - - - - - - -    Allocated - - - - -
                                                  A       B     %         A       B     %
graphFromEdges.line,n=100                    6.2 μs  5.2 μs  -15%     28 KB   17 KB  -39%
graphFromEdges.line,n=1000000                366 ms  315 ms  -14%    267 MB  160 MB  -39%
graphFromEdges.maxDAG,n=15                   3.0 μs  2.5 μs  -16%     13 KB  9.5 KB  -24%
graphFromEdges.maxDAG,n=1500                  91 ms   87 ms   -4%     95 MB   77 MB  -18%
graphFromEdges.rand,n=100,m=1000              43 μs   39 μs   -9%    105 KB   80 KB  -23%
graphFromEdges.rand,n=100,m=10000            531 μs  513 μs   -3%    879 KB  713 KB  -18%
graphFromEdges.rand,n=10000,m=100000          16 ms   15 ms   -4%     10 MB  7.8 MB  -23%
graphFromEdges.rand,n=100000,m=1000000       340 ms  323 ms   -4%    102 MB   78 MB  -23%
graphFromEdges.star,n=100                    6.0 μs  4.9 μs  -18%     28 KB   17 KB  -39%
graphFromEdges.star,n=1000000                466 ms  302 ms  -35%    267 MB  160 MB  -39%
stronglyConnCompR.line,n=100                  16 μs   15 μs   -6%     71 KB   60 KB  -15%
stronglyConnCompR.line,n=1000000             848 ms  735 ms  -13%    734 MB  628 MB  -14%
stronglyConnCompR.maxDAG,n=15                6.3 μs  5.7 μs   -8%     21 KB   18 KB  -14%
stronglyConnCompR.maxDAG,n=1500              202 ms  198 ms   -1%    121 MB  104 MB  -14%
stronglyConnCompR.rand,n=100,m=1000           72 μs   67 μs   -7%    171 KB  146 KB  -14%
stronglyConnCompR.rand,n=100,m=10000         816 μs  761 μs   -6%    1.1 MB  990 KB  -14%
stronglyConnCompR.rand,n=10000,m=100000       29 ms   27 ms   -7%     18 MB   15 MB  -13%
stronglyConnCompR.rand,n=100000,m=1000000    672 ms  645 ms   -4%    177 MB  153 MB  -13%
stronglyConnCompR.star,n=100                  15 μs   14 μs   -6%     71 KB   60 KB  -15%
stronglyConnCompR.star,n=1000000             852 ms  677 ms  -20%    734 MB  628 MB  -14%

@meooow25 meooow25 force-pushed the graph-from-edges branch 4 times, most recently from d300dc2 to a1ba2cd Compare June 27, 2025 19:06
@meooow25
Copy link
Contributor Author

Just realized that we can do 2 much more low-tech since it's just an Int 🤦

meooow25 added 2 commits June 30, 2025 23:10
* Replace `array` with `listArray` so we don't need to allocate
  `(index, element)` pairs.
* Return an unboxed `Maybe` from the binary search on GHC to avoid
  `Just` allocations.
* Return -1 from the binary search when the key is not found to avoid
  `Just` allocations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant