python / pyperformance

Python Performance Benchmark Suite
http://pyperformance.readthedocs.io/
MIT License
870 stars 175 forks source link

Pass --timeout flag to pyperf #354

Closed diegorusso closed 1 month ago

diegorusso commented 2 months ago

Add the --timeout flag to the run command. By default there is no timeout and user needs to specify it if they want it. I wanted to avoid to change the current behaviour. This addressed the issue https://github.com/python/pyperformance/issues/353

If you run the below command passing the timeout of 1 second, dask will fail and it will be reported at the end.

$ pyperformance run --debug-single-value -b float,go,mako,dask --timeout 1
...
...
Performance version: 1.11.0
Python version: 3.12.3 (64-bit)
Report on Linux-6.8.0-44-generic-aarch64-with-glibc2.39
Number of logical CPUs: 8
Start date: 2024-09-19 10:11:20.472768
End date: 2024-09-19 10:11:21.843698

### float ###
85.8 ms

### go ###
119 ms

### mako ###
9.77 ms

1 benchmarks failed:
- dask (Timed out after 1 seconds)
diegorusso commented 2 months ago

I may have sent you forth with bad information by suggesting that this would be easier in pyperformance, and maybe @vstinner is right that this would be better handled in pyperf.

We'd still need a patch to pyperformance to pass the --timeout value down to pyperf -- 80% of this PR is handling commandline arguments as it is, so not wasted effort.

Don't worry, that's the purpose of the review. I leave this PR open for now and implement the timeout in pyperf. Once that one is in, I'll fix this up to make use of the timeout.

diegorusso commented 2 months ago

I've addressed feedbacks on this PR and also raised a new one in pyperf: https://github.com/psf/pyperf/pull/205

diegorusso commented 2 months ago

Just to let you know that the pyperf PR has been merged (thanks @vstinner) so we have the timeout functionality there.

vstinner commented 2 months ago

Just to let you know that the https://github.com/psf/pyperf/pull/205 has been merged (thanks @vstinner) so we have the timeout functionality there.

You need a pyperf release first.

mdboom commented 1 month ago

Just to let you know that the psf/pyperf#205 has been merged (thanks @vstinner) so we have the timeout functionality there.

You need a pyperf release first.

I started one here

vstinner commented 1 month ago

Your PR should update pyperf to 2.8.

diegorusso commented 1 month ago

Your PR should update pyperf to 2.8.

I see that pyperf is unpinned so in theory I should not change anything in there. I have done a fresh install of pyperformance and in fact it installs pyperf 2.8

mdboom commented 1 month ago

Your PR should update pyperf to 2.8.

I see that pyperf is unpinned so in theory I should not change anything in there. I have done a fresh install of pyperformance and in fact it installs pyperf 2.8

I think you still need to update it here, which is the version that will actually run in the venvs.

diegorusso commented 1 month ago

Your PR should update pyperf to 2.8.

I see that pyperf is unpinned so in theory I should not change anything in there. I have done a fresh install of pyperformance and in fact it installs pyperf 2.8

I think you still need to update it here, which is the version that will actually run in the venvs.

Yes, you are right. I've just found it! :)

diegorusso commented 1 month ago

I'm currently running the tests because there is a small change in the way pyperf reports unit measure: https://github.com/psf/pyperf/commit/432c419cff97804de8c780e82d2a8562e977275a