-
Notifications
You must be signed in to change notification settings - Fork 66
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
Relative statistics #280
Relative statistics #280
Conversation
lib/benchee/statistics.ex
Outdated
@@ -37,6 +40,9 @@ defmodule Benchee.Statistics do | |||
mode: mode, | |||
minimum: number, | |||
maximum: number, | |||
relative_more: float | nil | :infinity, | |||
relative_less: float | nil | :infinity, | |||
absolute_difference: float | nil, |
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.
feedback on field names highly welcome :)
This is what it looks like now with absolute difference, I think it's cool but there might be room to improve:
|
d92ecb4
to
0097083
Compare
compiler was right about a couple of unused variable warnings :) |
CI is failing for windows... it rather seems like something is wrong with windows/fast function repetition or something as we seem to get the same measurements for map.flatten and flat_map and hence a failing ci on the absolute changes:
I don't have a windows install anymore. Yeha on setting up a VM and trying to reproduce. Sad. 😢 |
It sometimes fails on my VM... this is progress. |
Ok so now this actually gets rid off a significant Windows bug. Fingers crossed for passing build 🤞 |
43d8b7d
to
488a608
Compare
☝️ the build actually passed but I guess the owner change has caused it some problems 🤷♂️ |
This is done for 2 reasons: 1. support a new feature I really want to have before 1.0/as a little migration bait which is supposed to show the absolute difference to the reference scenario/fastest scenario. This is good to put performance gains into perspective. It's great if something is 100 times as fast, but if it only gains me 1 ms in a program that takes 100ms then it's probably not worth it 2. centralize the calculation of the multiplicator / lesser / more thing so that not every formatter has to implement that popular calculation by themselves. This also revealed an interesting case that I think we're not handling correctly in the formatters yet that happens when the average is 0. It took more code than I thought it'd but looking at it I also wouldn't know where to cut it. Input welcome specifically on the struct field names, half-way unsure there. I didn't document them yet as the other PR isn't merged yet. Other features (squashed because repeatedly resolving conflicts is tiring): * test running without any jobs does not blow up * new fast sample for quicker whole stack dev feedback * incorporate relative_more instead of impromptu calculations * Take care of incomparable values
Added new scenarios to the memory measuyrement brekaer test because now having to with 0 memory consumption is actually wanted.
We didn't scale our measured native time for determine_n to nanoseconds until now 😱 Impact is little, as to the best of my knowledge it only affects Windows systems and then only significantly with few runs... still this shouldn't have happend.
(partly to trigger build again so appveyor hopefully doesn't look stuck anymore)
3ea3e34
to
200dc8c
Compare
There is... no CI running? Have I accidentally broken everything? |
04ff897
to
0a8900f
Compare
This is done for 2 reasons:
little migration bait which is supposed to show the absolute
difference to the reference scenario/fastest scenario. This
is good to put performance gains into perspective. It's
great if something is 100 times as fast, but if it only
gains me 1 ms in a program that takes 100ms then it's probably
not worth it
thing so that not every formatter has to implement that
popular calculation by themselves. This also revealed an
interesting case that I think we're not handling correctly in
the formatters yet that happens when the average is 0.
It took more code than I thought it'd but looking at it I also
wouldn't know where to cut it.
Input welcome specifically on the struct field names, half-way
unsure there. I didn't document them yet as the other PR isn't
merged yet.
edit: now also includes output which you can see in a comment below