Skip to content
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

Add cabal benchmark integration #46

Closed
wants to merge 1 commit into from

Conversation

treeowl
Copy link
Contributor

@treeowl treeowl commented Aug 30, 2016

  • Add the benchmarks to fgl.cabal.
  • Disable AVL benchmarks that result in errors. Surely something
    needs to be fixed!

@treeowl treeowl force-pushed the cabal-benchmark branch 2 times, most recently from cf7227d to 95215cb Compare August 30, 2016 22:45
@treeowl
Copy link
Contributor Author

treeowl commented Aug 30, 2016

I've also added a benchmark to build up a very full graph using &.

buildFull' g n limit
| n == limit = empty
| otherwise = ([((), k) | k <- [0..n-1]],n,(),[((),k) | k <- [0..n-1]]) & buildFull' g (n + 1) limit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the first argument is just for a proxy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the first argument is the graph to start with. But I'm using the Int
parameter wrong, because we can't really use it right. I think probably the
best we can do is ignore it.

On Tue, Aug 30, 2016 at 7:37 PM, Ivan Lazar Miljenovic <
notifications@github.com> wrote:

In test/benchmark.hs
#46 (comment):

insNodePatricia :: Int -> Patricia.UGr
insNodePatricia = insNodes' empty

+{-# INLINE buildFull' #-}
+buildFull' :: DynGraph gr => gr () () -> Int -> Int -> gr () ()
+buildFull' g n limit

  • | n == limit = empty
  • | otherwise = ([((), k) | k <- [0..n-1]],n,(),[((),k) | k <- [0..n-1]]) & buildFull' g (n + 1) limit

I assume the first argument is just for a proxy?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/haskell/fgl/pull/46/files/95215cb25d346d910f7fb11dcc4dddd3e316a95d#r76899816,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzi_cbvK3lbHfN1UtAIiNVjs4pKJIXvks5qlL6jgaJpZM4Jw2_G
.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, no, ignoring it won't work either. I have no idea how to make
my benchmark work with microbench.

On Tue, Aug 30, 2016 at 7:41 PM, David Feuer david.feuer@gmail.com wrote:

No, the first argument is the graph to start with. But I'm using the Int
parameter wrong, because we can't really use it right. I think probably the
best we can do is ignore it.

On Tue, Aug 30, 2016 at 7:37 PM, Ivan Lazar Miljenovic <
notifications@github.com> wrote:

In test/benchmark.hs
#46 (comment):

insNodePatricia :: Int -> Patricia.UGr
insNodePatricia = insNodes' empty

+{-# INLINE buildFull' #-}
+buildFull' :: DynGraph gr => gr () () -> Int -> Int -> gr () ()
+buildFull' g n limit

  • | n == limit = empty
  • | otherwise = ([((), k) | k <- [0..n-1]],n,(),[((),k) | k <- [0..n-1]]) & buildFull' g (n + 1) limit

I assume the first argument is just for a proxy?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/haskell/fgl/pull/46/files/95215cb25d346d910f7fb11dcc4dddd3e316a95d#r76899816,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzi_cbvK3lbHfN1UtAIiNVjs4pKJIXvks5qlL6jgaJpZM4Jw2_G
.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I know what to do, I think. We need to run the same benchmark as many
times as it tells us, and hope the optimizer doesn't eat our shorts. Ugh.

On Tue, Aug 30, 2016 at 7:46 PM, David Feuer david.feuer@gmail.com wrote:

Actually, no, ignoring it won't work either. I have no idea how to make
my benchmark work with microbench.

On Tue, Aug 30, 2016 at 7:41 PM, David Feuer david.feuer@gmail.com
wrote:

No, the first argument is the graph to start with. But I'm using the Int
parameter wrong, because we can't really use it right. I think probably the
best we can do is ignore it.

On Tue, Aug 30, 2016 at 7:37 PM, Ivan Lazar Miljenovic <
notifications@github.com> wrote:

In test/benchmark.hs
#46 (comment):

insNodePatricia :: Int -> Patricia.UGr
insNodePatricia = insNodes' empty

+{-# INLINE buildFull' #-}
+buildFull' :: DynGraph gr => gr () () -> Int -> Int -> gr () ()
+buildFull' g n limit

  • | n == limit = empty
  • | otherwise = ([((), k) | k <- [0..n-1]],n,(),[((),k) | k <- [0..n-1]]) & buildFull' g (n + 1) limit

I assume the first argument is just for a proxy?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/haskell/fgl/pull/46/files/95215cb25d346d910f7fb11dcc4dddd3e316a95d#r76899816,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABzi_cbvK3lbHfN1UtAIiNVjs4pKJIXvks5qlL6jgaJpZM4Jw2_G
.

@treeowl treeowl force-pushed the cabal-benchmark branch 3 times, most recently from fe541f0 to 31a4659 Compare August 31, 2016 01:40
@treeowl
Copy link
Contributor Author

treeowl commented Aug 31, 2016

Ugh. Polykinded proxies weren't available till a bit late in the game. I doubt we care about running benchmarks on old GHC versions, so I'm just going to disable them for GHC < 7.8.

@ivan-m
Copy link
Contributor

ivan-m commented Aug 31, 2016

I'm OK with that.

On Wed, 31 Aug 2016, 11:41 AM David Feuer notifications@github.com wrote:

Ugh. Polykinded proxies weren't available till a bit late in the game. I
doubt we care about running benchmarks on old GHC versions, so I'm just
going to disable them for GHC < 7.8.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#46 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwXc5QP-DE_sjRjpER-vzHe3PfwlsGwks5qlNvYgaJpZM4Jw2_G
.

* Add the benchmarks to `fgl.cabal`.

* Disable AVL benchmarks that result in errors. Surely something
  needs to be fixed!

* Add benchmark for building a full graph with `&`.

* Enable GHC optimizations. Otherwise, none of the `RULES` or
  inlining the source code talks about can ever happen.

* Disable `cabal check`, which barfs on `-O2`.
@treeowl
Copy link
Contributor Author

treeowl commented Aug 31, 2016

All right. I think I've got the benchmarks doing what they're supposed to now. I produce as many graphs (of my chosen sizes) as microbench tells me to, labeling each of them differently, and force them all to normal form. I believe this is appropriate to the benchmark framework, and the optimizer's not smart enough to monkey around with it too terribly much.

@ivan-m
Copy link
Contributor

ivan-m commented Oct 5, 2016

This works for fixing the AVL edge benchmarks:

-insEdges' g n = let g' = insEdge (1, n, ()) g
+insEdges' g n = let n' = n - 1
+                    g' = insEdge (0, n', ()) g
                 in
-                  insEdges' g' (n - 1)
+                  insEdges' g' n'

@ivan-m
Copy link
Contributor

ivan-m commented Oct 5, 2016

I wonder if there's a nicer way of allowing benchmarks to be run on only newer GHC's than using CPP hackery... fake it by putting a minimum bound on base for benchmarks?

@treeowl
Copy link
Contributor Author

treeowl commented Oct 5, 2016

You'd also have to tell Travis not to try to build the benchmarks for earlier versions. I don't know much about that.

@ivan-m
Copy link
Contributor

ivan-m commented Oct 6, 2016

Right, didn't consider that. Let's go with this workaround of yours then whilst older GHCs are still supported.

@ivan-m ivan-m closed this in 8ebb465 Feb 15, 2017
@ivan-m
Copy link
Contributor

ivan-m commented Feb 15, 2017

I've pulled this in and tweaked it up a bit.

I remembered that I had a similar issue with Proxies for the test-suite and ended up rolling my own solution, which I've re-used for this, so no CPP hackery is required! \o/

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.

None yet

2 participants