Percentile Support

[jdhardy]

Fix #92 by adding support for calculating & displaying arbitrary percentiles, by using --benchmark-columns=p99.9. Only requested percentile columns are calculated and displayed. They are also preserved in CSV and JSON output formats if they are requested.

Had some issues running tox tests, but the core variants all passed.

[jdhardy]

Here's what the output looks like with --benchmark-columns=Min,Median,P99.9,Max,Ops:

================================================= test session starts ==================================================
platform darwin -- Python 3.6.3, pytest-3.2.3, py-1.4.34, pluggy-0.4.0
benchmark: 3.1.1 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /.../tests/perf, inifile: pytest-perf.ini
plugins: cov-2.5.1, benchmark-3.1.1
collected 3 items

tests/perf/test_Perf.py ...


-------------------------------------------- benchmark 'test_benchmark': 3 tests ---------------------------------------------
Name (time in ms)                  Min             Median               P99.9                 Max                OPS
------------------------------------------------------------------------------------------------------------------------------
benchmark[2000-xxxxxxxx]        9.0091 (1.0)      11.9048 (1.0)       43.1680 (1.0)       43.1680 (1.0)      78.7381 (1.0)
benchmark[200-xxxxxxxx]        11.8196 (1.31)     12.3910 (1.04)     401.4592 (9.30)     401.4592 (9.30)     47.8764 (0.61)
benchmark[200000-xxxxxxxx]     33.7439 (3.75)     37.7569 (3.17)      74.0722 (1.72)      74.0722 (1.72)     25.5273 (0.32)
------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean
============================================== 3 passed in 14.32 seconds ===============================================

[ionelmc]

This looks pretty good. I still need to look over the code a bit but there is one thing that makes me a bit wary: the fact that the pXX.X column only gets in the saved data if it's active in the display (via --benchmark-columns).

This would be a problem if the user doesn't save the data (via either --benchmark-save-data or --benchmark-columns=pXX.X) and then tries to look at previous run data with a new or different pXX.X. This is basically my fault here, I shouldn't have implemented --benchmark-save-data (the include_data in the internals) so I wonder if it's time to remove it, always include full data and just let users live the few MBs of json ...

[ionelmc]

I would love to hear some opinions on this problem.

[jdhardy]

Storing the percentiles was one part I wasn't sure about. Perhaps a new --benchmark-save-columns option for those that don't want to/can't store all data?

[ionelmc]

So I've been thinking about this again, and I want this feature. But I was these stats to be always saved.

@jdhardy Would you think people would ever want anything else besides p90, p95, p99, p99.9 and p99.99?

[jdhardy]

*Possibly* p99.999. I've never seen anything else used around here, anyway. Seems like that's enough to start with and see if anyone requests others.

…

On Thu, Feb 8, 2018 at 3:03 PM, Ionel Cristian Mărieș < ***@***.***> wrote: So I've been thinking about this again, and I want this feature. But I was these stats to be always saved. @jdhardy <https://github.com/jdhardy> Would you think people would ever want anything else besides p90, p95, p99, p99.9 and p99.99? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#93 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGmdQ0SFDz7FFUxb7thjVTup4QBl1i5ks5tS31BgaJpZM4QWxt9> .

[ionelmc]

Oh damn it's almost a year.

So now I'm thinking about a compromise ... eg, always compute p90, p95, p99, p99.9 and p99.99 and compute and store on demand anything else. Tho it doesn't seem like anyone wants anything else other than p90, p95, p99, p99.9 and p99.99, so just remove the on-demand thing? What do you think @jdhardy? (just asking for opinion, not pr rework)

[jdhardy]

I'm actually (finally) circling back on this in the next week or two. I'm fine with always doing p90/95/99/99.9/99.99 to start with and then seeing if anyone wants anything else. That should cover the vast majority of use cases.

[ionelmc]

@jdhardy hey, you still wanna work on this?

[jdhardy]

Yes, and when I get back from vacation next week I'll see when I can fit it into my schedule.

…

On Thu, Jan 3, 2019 at 10:42 AM Ionel Cristian Mărieș < ***@***.***> wrote: @jdhardy <https://github.com/jdhardy> hey, you still wanna work on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#93 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGmdWPDplf1vsjucMautGLclo6Z4PVmks5u_k8SgaJpZM4QWxt9> .

[jdhardy]

Yikes. I haven't had a chance to look at this for a while, and probably won't for a while longer (I'm no longer working on that project). Feel free to close it, and apologies for not being able to follow through.