-
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
First shot at an appveyor configuration #195
Conversation
It builds 🎉 🎉 🎉 But apparently it's super slow and hence a bunch of tests fail 😢 https://ci.appveyor.com/project/PragTob/benchee/build/build19
For reference, even elixir 1.4 with erlang 18.3 on travis took 3.8 seconds... |
So that leaves us at an every dangerous crossroad:
There's also some other stuff which seems to fail that's not entirely related to fast/slow - will have to look another day though :) |
This is great! Well, great-ish. I'm only seeing failures related to speed things. Like, either the expected counts for run times don't match, or it's not printing the fast function warning, which we're expecting to see in the captured output. As to a solution, I don't think we should make the existing performance tests more lenient. I think the two best options are:
Personally, I think the cases where we would make a change that doesn't fail one of these performance tests on Linux but would make benchee noticeably slower on Windows are so rare that we don't need to worry about it. Frankly, I don't think it would ever happen, but I can't say that for sure. That's why I'd lean towards option 1, tagging these tests to be skipped on Windows machines. |
the regex thing still has me puzzled... it refutes that |
Omg... we actually print the fast warning... but WHY? It's slow in the measurement... it's weird:
(I mean look at the average it's milliseconds) I have no idea? Hopeful insight from @OvermindDL1 ? |
Is it printed to stderr instead of stdout or so? (/me has not looked yet) |
@OvermindDL1 problem is that we print the warning at all, which shouldn't happen at all as the benchmark isn't that fast - I vaguely remembered that there was something that the native time unit might change but couldn't find it again. Also |
Good news! I'm able to relatively reliably reproduce all/most of this errors - my last test run also had 13 errors and also the very weird "fast" errors. Also the errors about being too slow. Mind you this is the exact same machine I often develop on with linux. So either elixir/erlang performance on Windows is that bad or it's Windows itself ;P Funnily enough, I also get the fast warning when I just run
(seems to be about half the speed of what it does on Linux) |
Grumble mumble windows grumble mumble.... TLDR; the code is right 19> Data1000 = lists:seq(1, 1000).
[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,
23,24,25,26,27,28,29|...]
20> Data10000 = lists:seq(1, 10000).
[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,
23,24,25,26,27,28,29|...]
21> Data100000 = lists:seq(1, 100000).
[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,
23,24,25,26,27,28,29|...]
22> timer:tc(lists, map, [Succ, Data1000]).
{0,
[2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,
24,25,26,27,28|...]}
23> timer:tc(lists, map, [Succ, Data10000]).
{15000,
[2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,
24,25,26,27,28|...]}
24> timer:tc(lists, map, [Succ, Data100000]).
{77999,
[2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,
24,25,26,27,28|...]} Seems like the native time unit in Erlang on Windows isn't Microseconds but Milliseconds. That's why it doesn't register. There's something on time correction in the erlang docs - but before we all go mumbo jumbo -
--> we either gotta run more juicy benchmarks or also exclude these or those specific checks on Windows. Sigh. Side mark, it's also documented in the elixir System docs where I feel it should be mentioned that you can also just use timer.tc - I should send a patch and see what they say. |
Woah! That's a pretty huge difference between Windows and Unix!! Happy you got to the bottom of it! 🎉 |
Interestingly, the native unit on Linux seems to be nanoseconds. I believe I tried this out once - but probably didn't take monotonic time 🤦♂️ Might be worth another shot, it means we have to replace
edit: leaves the question if we use the above workaround (converting times until the times are the same) or if we're naughty and call
|
Just tested here, Windows and Linux report VERY different time values, this really seems like a bug to me, maybe report it? At the very least not reporting the temporal range of the return is a bug. |
* =~ seems to print the text that was matched against while Regex.match? doesn't
Excluded theses tests or the one problematic assertions and did it on my windows machine --> high hopes that this passes now :) @OvermindDL1 What exactly do you mean by different values? You mean return values from
Not sure how the OTP team rolls but wanna report these one of these days :) |
It passed 💃 |
For grabbing time like monotonic time. The extra accuracy on linux is welcome, but we need to know what that accuracy is. ^.^; Thus I'd think an extra function to get the timing accuracy of the BEAM VM on the current system would be both very nice and backwards compatible. |
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.
This all looks good to me!! I'm happy we got to the bottom of why that's happening (and that you included that history in comments 😉)
Fixes #79