psf / pyperf

Toolkit to run Python benchmarks
http://pyperf.readthedocs.io/
MIT License
782 stars 77 forks source link

Add support for profiling benchmarks #134

Closed mdboom closed 2 years ago

mdboom commented 2 years ago

This should make it much easier to collect profiles for benchmarks in the pyperformance suite.

Implements #133.

TODO:

vstinner commented 2 years ago

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?

mdboom commented 2 years ago

Tests don't pass.

Noted. Wanted to get some feedback about the general concept here first.

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?

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.

mdboom commented 2 years ago

I'm not sure I understand the request.

The temporary file only comes into play when using bench_process. In that case, the temporary filename is generated in parent process and passed to the child process being benchmarked specifically to make it easier to clean up because it could crash. (The temporary file is deleted whether the worker process succeeds or fails).

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.

vstinner commented 2 years ago

cc @corona10

vstinner commented 2 years ago

cc @pablogsal

corona10 commented 2 years ago

I will left review by this weekend cc @vstinner

corona10 commented 2 years ago

I am going to release the new version of pyperf in 7days after this PR is merged. cc @vstinner

mdboom commented 2 years ago

Please update the following documentation.

https://github.com/psf/pyperf/blob/6eb6de7427c5ac8939a21c0c68014be1167847de/doc/cli.rst#pyperf-timeit

@corona10: I already did that in this PR. Is there something specific missing there that you'd like to see?