Add complete Pipes.Prelude benchmarks.

[archblob]

Previously there were two modules for this single benchmark because I did not know that running cabal bench without a benchmark name will run all benchmarks and I had in mind to do an all-benchmarks benchmark module.

I lowered the minimum Cabal version to 1.10, and that's the minimum version required for the Default-Language option.

I grouped enumFromTo and each but I really don't like the name I gave the group.

runIdentity is used a lot and I really don't know if I should make a shorter synonym or not.

I left a few function from Pipes.Prelude out because I'm not sure how to benchmark them.

Please take a look when you have the time and let me know if the benchmarks are appropriate, and other stuff.

[Gabriella439]

Are you sure you need the runIdentitys at all? Can't you just do whnf without removing the Identity, since it is just a newtype?

You can call the enumFromTo/each benchmark "enumFromTo vs. each".

Also, instead of whnf (forceMaybeId m), you can just do nf, which does the same thing. The DeepSeq instance for Maybe already does what you want.

You don't call preludeBenchmarks more than one time, so there you don't need to make it a function of vMax and benchEnum_p arguments since those will never change. You can therefore float benchEnum_p to a pure top-level declaration like this:

bencEnum_p :: Producer Int Identity ()
benchEnum_p = enumFromTo 1 defaultMax

... and also float out vMax, too:

vMax :: Int
vMax = defaultMax

Then you can change preludeBenchmarks so that it doesn't take any parameters:

preludeBenchmarks :: [Benchmarks]

[archblob]

runIdentity is not needed, but I observed that I don't get high outlier counts when it is there, on the same quiet system.

preludeBenchmarks takes two arguments because I thought I'd make available the possibility to pass vMax as an optional argument to the benchmark, but I don't know, what do you think ?

[Gabriella439]

I didn't know that you could pass arguments to the benchmark. In that case it would be very useful since then we could pass a smaller vMax for a very quick benchmark and a larger vMax for the final benchmark before release.

[archblob]

Criterion exports a parseArgs function. I did not try to implement this yet but I thought it should work and as a result preludeBenchmarks has to take at least one argument.

[Gabriella439]

So if you type cabal bench --help it lists this:

Usage: cabal bench [FLAGS]

Flags for bench:
 -h --help                        Show this help text
 -v --verbose[=n]                 Control verbosity (n is 0--3, default
                                  verbosity level is 1)
    --builddir=DIR                The directory where Cabal puts generated
                                  build files (default dist)
    --benchmark-options=TEMPLATES give extra options to benchmark executables
                                  (name templates can use $pkgid, $compiler,
                                  $os, $arch, $benchmark)
    --benchmark-option=TEMPLATE   give extra option to benchmark executables
                                  (no need to quote options containing spaces,
                                  name template can use $pkgid, $compiler, $os,
                                  $arch, $benchmark)

So I think you just pass arguments using --benchmark-options and parse them the same way you parse arguments to an ordinary Haskell program.

[archblob]

I made adjustments and amended the commit. I didn't notice but the benchmarks module failed to build because when I rebased on current master I failed to notice that you generalized fold definitions. Now it builds.

I saw that arguments are passed with --benchmark-options and tried to get them with getArgs just as a test, a few days ago, but somehow they were passed directly to criterion. :-P, or maybe I was not paying attention and made a mistake, either way, this is possible and I'll add the option.

[Gabriella439]

Great! In that case just add a comment to the code explaining why runIdentity is necessary when using criterion.

Also, I don't think you need an INLINABLE for enumFromTo. INLINABLE only makes a difference if you export the function from the module, since the only thing INLINABLE does is ask ghc to guarantee that it will include the code for that function in the interface file for that module.

It might be worth making msum a little more "point-ful". It's obvious what it does from the type signature but the implementation is hard to read.

You can make the last two benchmarks for the "unfolds" section more consistent with the others by rewriting them to use drain like this:

    ...
    , bench "yieldIf"    $ whnf (drain . (P.yieldIf even <\\)) benchEnum_p
    , bench "chain"      $ whnf (drain . (P.chain return <\\)) benchEnum_p
    ...

[archblob]

I made changes and also now you can set vmax at runtime.

It turns out that this was not possible with the available criterion 'machinery' , so I used getArgs to parse the custom options and then the benchmarks run using withArgs.

While running with a lower maximum value I no longer see any difference using runIdentity, so I don't know if I should delete it.

Also I consistently see about a half millisecond faster enumFromTo when INLINABLE is there.

[Gabriella439]

I would keep the runIdentitys as long as you document why they are there.

The INLINABLE result surprises me, but it is real, so keep the annotation.

I don't see any other improvements, so if you want to clean up the commit then I will merge it in.

[archblob]

I cleaned the commit and added a couple of comments.

I'm planning on working to benchmark the Pipes.Lift module next, but if you think that I should continue with something else or that I should not do it on a module basis, please comment.

[Gabriella439]

Oh yeah, I forgot about Pipes.Lift. You can definitely do those. Do you want me to merge right now or wait until after you finish every module? I'm fine either way.

[archblob]

You can merge this now and i'll open another pull request for review when i'm done with Pipes.Lift.