Add support for profiling benchmarks #134
Conversation
|
Tests don't pass. How does merge_profile_stats() work? Does it compute the average of N processes timings? Or does it accumulate time? Is it more reliable than running a single process? |
Noted. Wanted to get some feedback about the general concept here first.
Yes, it just accumulates timings from multiple profiling runs. It produces more accurate results, in the same way that running the benchmarks multiple times does. |
pyperf/_process_time.py
Outdated
| max_rss = 0 | ||
| range_it = range(loops) | ||
| start_time = time.perf_counter() | ||
|
|
||
| if profile_filename: | ||
| with tempfile.NamedTemporaryFile(suffix=".profile", delete=False) as fh: | ||
| temp_profile_filename = fh.name |
There was a problem hiding this comment.
IMO tempfile.mktemp() would be better than using NamedTemporaryFile() here.
There was a problem hiding this comment.
Would it be possible to create the temporary file in pyperf/_manager.py and pass it to worker processes? A worker is more likely to crash, and so it's harder to make sure that the temporary file is removed in case of a crash.
Maybe the profiling data could be written into a pipe by the worker, and the manager would be responsible to merge data? I don't really if pyperf uses pipes or not on Windows.
|
I'm not sure I understand the request. The temporary file only comes into play when using For other kinds of benchmarks, there is no temporary file involved -- each child worker process merges their results directly into the output file. That has a separate problem in that there is a race condition if multiple workers update that file at the same time, but the whole design here is to not benchmark things in parallel, so that should be ok. This certainly could use a pipe to communicate all the profiling data to the parent process -- all platforms already use that to communicate benchmark results from the worker processes. But it would add complexity to a bunch of places since the "protocol", which right now dumps things directly into the master benchmarking results, would have to split things out into separate files for the profiling results. |
|
cc @corona10 |
|
cc @pablogsal |
|
I will left review by this weekend cc @vstinner |
|
I am going to release the new version of pyperf in 7days after this PR is merged. |
Co-authored-by: Dong-hee Na <[email protected]>
@corona10: I already did that in this PR. Is there something specific missing there that you'd like to see? |
This should make it much easier to collect profiles for benchmarks in the pyperformance suite.
Implements #133.
TODO: