-
Notifications
You must be signed in to change notification settings - Fork 49
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
Introduce basic benchmarks of the library. #64
Conversation
Introduce basic benchmarks of the log mechanism that allows testing the overhead of the each feature added. Example output: Dump 10k messages (in a forked process): baseline 0.001114674s stdout<string 0.005194026s stdout<text 0.004967748s stdout<bytestring 0.002918046s stderr<bytestring 0.01753503s {stdout<>stderr}<bytestring 0.017461043s stdout<format<message 0.009314225s stdout<format<message{with callstack} 0.009744045s stdout<format<message{with callstack:5} 0.018102189s stdout<format<message{with callstack:50} 0.018013676s stdout<bytestring<format<message 0.009570143s stdout<bytestring<rich-message<upgrade<message 0.134770134s Currently benchmarks are implemented outside of the main application as I had trouble to make `cabal new-configure` find the build plan without `--allow-newer`. Also these benchmarks may introduce dependencies on the other logging libraries in the future. At this point of time only the simplest benchmarks were introduced. This benchmarks are testing sequencial dump of the logs with different features added atop. Theoretically current benchmarks may be moved to the application and criterion can be used for measuring time. However I planned to make more long running benchmarks where criterion may not shine, and such benchmarks may make CI time much larger for not purpose.
@qnikst Thank you for these benchmarks! It's kinda strange that outputting I think it's okay to keep these particular benchmarks in separate package. They might be huge, so no need to move into existing package. I'm okay to not use |
@@ -0,0 +1,32 @@ | |||
name: co-log-benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please update cabal.project
file and add name of this package there?
And also update .travis.yml
file for stack
section to build this package on CI as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, will do
@@ -0,0 +1,2 @@ | |||
import Distribution.Simple | |||
main = defaultMain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that Setup.hs
is not needed for cabal
, it works without this file pretty fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that require you to have cabal-install. But with Setup.hs, you may install library without cabal-install
that what most of the distributions do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qnikst Could you, please, elaborate on this? How is it possible to install Haskell library without cabal-install
and why one might want to do this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can always run:
ghc Setup.hs
./Setup configure
./Setup build
./Setup install
It will not install any dependency, but you don't need any additional tool to do that. Source distribution like Gentoo
, NixOS
do that, and other when you prepare a binary package. Because dependencies and package management is solved by the special tools.
Nothing -> pure () | ||
Just f -> f | ||
|
||
-- | Measure the running time of the process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the documentation!
co-log-benchmark/LICENSE
Outdated
@@ -0,0 +1,20 @@ | |||
Copyright (c) 2018 Alexander Vershilov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, how should I update the LICENSE if I want to do something with the benchmarks (like, refactor or add new ones)? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh.. that was autogenerated.
Seems |
I hope I've fixed everything that was needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@qnikst Thank you very much for contributing these benchmarks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. But then I've noticed couple more things that should be fixed
@@ -3,6 +3,7 @@ resolver: lts-12.9 | |||
packages: | |||
- co-log | |||
- co-log-core | |||
- co-log-benchmark |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need co-log-benchmark
to .travis.yml
here:
https://github.com/kowainik/co-log/blob/76964369c38152b2ad10bd3c452e0e7a0a90dfb9/.travis.yml#L79-L80
@@ -23,10 +22,11 @@ executable co-log-features-bench | |||
main-is: Main.hs | |||
-- other-modules: | |||
-- other-extensions: | |||
build-depends: base >=4.10 && <4.13, | |||
build-depends: base-noprelude >=4.10 && <4.13, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh. Prelude
in the co-log
package is now in other-modules
so base-noprelude
trick won't work. Anyway, you didn't used relude
before in co-log-benchmarks
so I don't expect that addition of relude
is required... Need to see CI failure to check what's wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to either use Prelude trick:
import Data.Semigroup
import Prelude
so semigroup will be imported in all GHC versions, or use relude
.
( main | ||
) where | ||
|
||
import Data.Time.Clock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import Data.Time.Clock | |
import Data.Semigroup ((<>)) | |
import Data.Time.Clock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope that should fix CI error. But the last commit with base-noprelude
should be reverted. Alternative, you can add Prelude
module to the co-log-benchmarks
packages
This reverts commit b39e6e3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, CI passes and it looks good 👍
Thank you again for the contribution!
Introduce basic benchmarks of the log mechanism that allows
testing the overhead of the each feature added.
Example output:
Currently benchmarks are implemented outside of the main
application as I had trouble to make
cabal new-configure
findthe build plan without
--allow-newer
. Also these benchmarksmay introduce dependencies on the other logging libraries in the
future.
At this point of time only the simplest benchmarks were introduced.
This benchmarks are testing sequencial dump of the logs with different
features added atop. Theoretically current benchmarks may be moved
to the application and criterion can be used for measuring time.
However I planned to make more long running benchmarks where criterion
may not shine, and such benchmarks may make CI time much larger for
not purpose.