-
Notifications
You must be signed in to change notification settings - Fork 139
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
Benchmarks based on CPU counters #426
Conversation
21006c3
to
762ec6d
Compare
Now this PR contains benchmarks which use Sadly it's not possible to run them on github CI. |
I don't have a Linux machine to test, but otherwise have no objections. Looks interesting. |
I've tried it on my setup with OpenSUSE and I get bunch of errors. Looks like CPU counters are not readily available on all Linux distros without extra configuration. I am a little skeptical about adding such benchmark directly to
|
First of all we can (edc9083) and do build these benchmarks on CI. We just can't actually run them I think splitting benchmarks means it will become even more complicated to run. Probably to the point where only I can run them. CPU counters are security sensitive with "specter" and all. So problems with access will happen. Hopefully they could be mitigated with better documentation. But I never encountered such problems (nixos, fedora37) so I can;t debug them What is |
Same case was for me. I was able to build them, just not run them. Which makes me question usability of the suite for your average contributor.
I really don't think making a core Haskell package depend on some third party C library that is only usable on some small subset of operating systems is a good idea, even if it is not buildable by default. Splitting it out into a separate package is the only thing that comes to mind as a decent solution to this. Also, I don't understand why would it be more complicated to run? Benchmarks are only useful for people who are making changes to Just to be clear, I am not against adding the benchmarks to the repo, because I do see value in such benchmarks. I am just against adding them to the main package. I'd rather see something like
That is already the case. Out of three maintainers you are the only one who can run them.
I can't debug them either since I've never used papi. Since you asked, I'll post the relevant output, but honestly, I don't have enough time to debug this issue any further.
|
Or maybe I don't quite understand what exactly are you proposing. One problem is benchmarks are not accessible from outside of vector package. So we have to either copy them, or split all benchmarks into separate package
|
We could export them as a public sub-library. |
Since they are require libpapi and only works on linux they're protected by cabal flag and have to be enabled explicitly PAPI benchmark couldn't be built on GHC 8.0
This is third attempt. PAPI based benchmarks are placed into separate package. They still require specifying cabal flag in order to not break build for anyone who don't want to run them. I also added |
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.
Beautiful!
We don't run benchmarks with any sort of regularity. In part because it's difficult to get repeatable result, it require dedicated PC and all measurement must be done on same hardware. So tracking performance regressions is hard. I run into this problem while working on size hints. Another way is to use CPU counters to count number of instructions. It's fast and hopefully reproducible and could possibly run on CI.
This PR is prototype. It sort of works, very rough on edges and right now isn't very deterministic. Apparently GC could kick in and skew results. I need to study this more and to figure out how to ensure repeatable measurements.
If everything works out I want to split it into separate package so far it's more convenient to develop it as part of vector.
P.S.
-O1
could be up to 3× slower than-O2
in our benchmarks but sometimes it's on par.