Closed headius closed 9 years ago
My big worry is the magnitude switching between lines. It's easy to miss the magnitude (even though you did reserve it a whole column) when looking at the numbers. What I'd prefer is to look at all the numbers and decide "ok, most of these are above 1000, we should display them all in kilo" so that we don't have switching between lines.
@evanphx Any possibility of getting rid of '1000000000 in' from this line. Does that convey more info than rate with total time (e.g. it is derivable and I never care about this number)?
Apologies for jumping into this bug but it is about formatting :)
@enebo Which line?
978.344 (± 5.1%) i/s - 4.883k in 5.006s
978.344 (± 5.1%) i/s over 5.006s
Unfortunately we can't pick a single scale for all numbers because they display their results immediately after executing. We'd have to wait and do a single report, and I think that hurts usability.
The scale changing concerned me at first, but I've had no trouble at all reading these suffixes. We're used to doing that with so many other aspects of computing...free space, time since last post (5s ago, 5m ago)...I don't see it as a problem.
And at any rate, it's MUCH easier to read 264.368M than 264368526. The latter causes me many more headaches than a changing suffix would.
Ah! The in X.Ys
is just for full disclosure. You're right, it doesn't convey much info other than to provide a quick sanity check that the test actually run the proper amount of time. In fact, benchmark/ips really needs to check those times and blow up if they're too far from the desired runtime.
Another case for suffix being more readable than exact value: it's harder to reasonably align columns if some results are 1 char wide and some results are twelve chars wide. This makes all results consistent width, followed by a scale suffix. Easier! :-)
@evanphx yeah ultimately I don't care about 5.0002s either but I would care if the delta from a desired 5s run was too large.
@headius Yeah, I can absolutely see the suffix being easier on those large numbers. I'm probably being a little overprotective. How about adding an option called ":human" that is on by default and when set, uses the prefixes. That at least gives people an easy out.
I thought about adding an option and I agree it would be good to have a "raw" form. I'll do as you suggest.
@enebo I agree that the in X.Y
ends up being noise. I'll change it so that it's suppressed unless the value is 5% outside the desired runtime.
@evanphx I don't see any options for IPS other than those passed into the Benchmark.ips method. I have a patch to add them there (https://gist.github.com/headius/61c09c2c5b8574b30df4). Is this what you meant? My gut is telling me this should be an option you can toggle without changing the benchmark, though.
That patch is fine. You could add a Hash at Benchmark::IPS.options
that contains the defaults so people can do Benchmark::IPS.options[:format] = :raw
.
I moved the option out of the ips() call and solely into Benchmark::IPS.options. It just didn't feel right to have the benchmark hardcode its format.
@headius Ok, you need to remove the other @format
stuff in there too then.
Oops, silly me. Removed the rogue bits and force pushed.
@headius one more fix and we'll be there!
Yeesh...guess I have too many distractions today. Re-pushed.
I rebased it again against my latest commit and committed it. Thanks!
The dot notation for the numbers is really throwing me off, but I also can't seem to convert to raw. I've tried passing it to the block object, as a argument to the #ips method, and more. How am I supposed to not use this new format for numbers?
Not only can I not find the way to set it, I'm fairly sure there is no way to set this value. There are no tests for this either.
@krainboltgreene You can set it back to raw by adding Benchmark::IPS.options[:format] = :raw
Here's the outputs before and after. I was always having trouble reading ips counts when they get into the hundreds of thousands or higher, and the stddev alignment thing was just bugging me. The ips scaling change also helps keep alignment consistent.
The precision changes warrant some justification. For the time elapsed, I figured that anything lower than ms is not useful for an "iterations per second" benchmark, and the only times shown are all around 5s range. For i/s, anything scaled 1/1000 is going to be in the neighborhood of benchmark noise. If something can do 50M iterations in 5 seconds, that's 10M per second, and 1000 iterations (the 1/10000 lost precision) takes 0.1ms. The error rate for that benchmark is +/- 1000 iterations, or 0.0002%. Acceptable to have good, readable results.
BEFORE:
AFTER: