-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move Benchmarks From Tools/ to the pyperformance Repo #97680
Comments
Tool/scripts/var_access_benchmark.py |
Why put them somewhere else? These are used during CPython development to quickly test the effect of changes. If you put them in another repo, they just get harder to access — I would likely never use them again or would have to keep a local copy. IIRC the original purpose of the Tools directly was as a convenient place for us to keep scripts that we use during development. |
I created the “faster Python” project and we definitely don’t use these to assess performance improvements. |
Regardless of the value of keeping these in the Tools directory, adding these micro benchmarks to pyperformance is still likely worth doing. (FWIW, I have no real interest either way in the ongoing Tools/ cleanup.) |
I concur with Eric's comment. Yeah, accessibility from some people can be degraded but I think that overall gain will be positive if we move them into pyperformance projects and I believe that we should reduce fragmentation as possible. |
If someone submits a PR to pyperformance can they please link to it here? |
I proposed #98214 to just remove |
That's a micro-benchmark. I don't think that it belongs to pyperformance which tries to be a collection of macro benchmarks. Well, if someone wants to move it there, go ahead. In the past, I created https://github.com/vstinner/pymicrobench which is a collection of CPython micro-benchmarks. If someone is interested by microbenchmarks, maybe it should be moved somewhere else. Usually, I only run a micro-benchmark once to prove that a change makes a function faster. But later, I don't use them anymore. If possible, I would prefer to move var_access_benchmark.py outside CPython source code, since I'm trying to remove Tools/scripts/: see issue #97669 :-) |
stringbench is also documented as being a microbenchmark:
I'm not sure if pyperformance is a good fit for it. |
I would have commented sooner had I known that this was about to happen. It would have been helpful if someone had commented in my PR to I believe it was clear from the extensive discussion about the shortcomings of the current GIL in faster-cpython that the That said, I do understand that performance scripts do not really have a place in the cpython source code repo, but unfortunately I do not have the time to rework the updated @vstinner Victor : I am sorry to hear that your own micro-benchmarking scripts are also languishing. Despite an overall good focus on cpython performance improvements, unfortunately from the evidence of the above referenced discussion, the lack of response to your appeal for someone to take on your own code and this issue/PR removing performance scripts do suggest that the focus on cpython performance improvements is somewhat narrow and clique-esque. Ah well - we can but try, |
Note: the above is reference to #101853 that I just merged (after being open for a year) which removed
Apologies, I'll ping next time this sort of thing comes up. (btw we wouldn't be allowed to merge your PR because it looks like the CLA isn't signed.)
Yep, in general, benchmarks are much more useful in pyperformance, because they're run often and people pay attention to their results. The code hasn't disappeared, it's in the 3.12 and older branches, and in Git history, so can still be referred to when/if adding to pyperformance. And of course, if there's consensus we can easily revert/re-add bits of #101853. |
(See https://discuss.python.org/t/remove-outdated-tools-scripts-scripts/19571/10.)
The Tools directory currently has some benchmarks in it:
It would probably make sense to move them to the pyperformance repo.
CC @rhettinger @gvanrossum
The text was updated successfully, but these errors were encountered: