Feature cProfile #59
Feature cProfile #59
Conversation
| @@ -168,6 +169,7 @@ def __init__(self, fixture, iterations, options): | |||
| self.group = fixture.group | |||
| self.param = fixture.param | |||
| self.params = fixture.params | |||
| self.cprofile_stats = fixture.cprofile_stats | |||
|
|
|||
| self.iterations = iterations | |||
| self.stats = Stats() | |||
There was a problem hiding this comment.
Now it becomes really obvious that I picked a confusing name for these two stats attributes. If you have naming ideas let me know (eg: what if it's would be rawstats or data? or maybe rename BenchmarkStats to BenchmarkData and so on?)
There was a problem hiding this comment.
Yes it's a bit confusing. What about
BenchmarStats -> BenchmarkMetadata
and
Stats -> BenchmarkStats?
And you can probably keep self.stats.
There was a problem hiding this comment.
Sounds good, but we should strip the „Benchmark” prefix, no sense to have it everywhere (obvious the stuff is going to be about benchmarks right?).
|
This saves the cprofile top ten into the storage right? I was wondering, what if we'd save the full data (so that the user can sort differently later, without re-running)? |
|
Yes it saves top ten function according to current sorting column. I would rather keep it limited because there can be hundreds to thousands functions and usually you are not interested in the most of them. Also, as we discussed in the team, we use mostly |
|
What if we's save just the top-10 for all the cols? That can't be that much. It's just that usually I have very slow suites and it's a pain in the ass just to rerun everything cause you wanted to see a different column. I'd expect it's the same for other people? |
|
Ok, I will think about the best way to have all needed information. |
|
I updated the code so we now have stored top ten functions for all columns (not redundant). It is still ordered by |
| @@ -215,6 +215,13 @@ def pytest_addoption(parser): | |||
| help="Fail test if performance regresses according to given EXPR" | |||
| " (eg: min:5%% or mean:0.001 for number of seconds). Can be used multiple times." | |||
| ) | |||
| group.addoption( | |||
| "--benchmark-cprofile", | |||
| metavar="COLUMN", default=None, | |||
There was a problem hiding this comment.
This option should have some validation.
| @@ -196,7 +199,8 @@ def __getitem__(self, key): | |||
| def has_error(self): | |||
| return self.fixture.has_error | |||
|
|
|||
| def as_dict(self, include_data=True, flat=False, stats=True): | |||
| def as_dict(self, include_data=True, flat=False, stats=True, | |||
| cprofile_sort_by="cumtime", cprofile_all_columns=False): | |||
There was a problem hiding this comment.
I would try to reduce the cprofile arguments to a sole cprofile argument (cause this call boiler plate is all over the place). If it's specified it means we want to display cprofile stats (sorted by column). If it's not specified it means we want all the columns for storage.
Eg:
.as_dict(cprofile=getoption(...))if we display the stats in table.as_dict()if we generate json for storage
| stats_columns.insert(0, cprofile_sort_by) | ||
| for column in stats_columns: | ||
| cprofile_functions.sort(key=operator.itemgetter(column), reverse=True) | ||
| for cprofile_function in cprofile_functions[:10]: |
There was a problem hiding this comment.
Maybe default to 25 rows? Just in case we need more info later.
| @@ -207,6 +211,20 @@ def as_dict(self, include_data=True, flat=False, stats=True): | |||
| (k, funcname(v) if callable(v) else v) for k, v in self.options.items() | |||
| ) | |||
| } | |||
| if self.cprofile_stats: | |||
| result["cprofile"] = [] | |||
There was a problem hiding this comment.
I would use an intermediate variable (eg cprofile = result["cprofile"] = []) for the loops bellow.
|
I've worked on your suggestions. It should be ok right now. Please review and merge. |
Current coverage is 60.92% (diff: 73.13%)
|
|
Alright, thanks for these changes, I know few people have been asking for the cprofile stats. |
Hello again,
this PR implements addition of
cProfiletopytest-benchmark. WithcProfilewe will know what are the most demanding subfunctions of benchmarked function. When you runpy.testwith option--benchmark-cprofileyou will get top 10 functions of every benchmarked function as you can see here:Those statistics are also saved to storage and you can compare them.