commands: fast path for --sh, replace sort|uniq with sort -u#3423
commands: fast path for --sh, replace sort|uniq with sort -u#3423native-api merged 2 commits intopyenv:masterfrom
Conversation
There was a problem hiding this comment.
The declared time gain is quite small, can be within the statistical discrepancy (with your scripts, I got some unstable benchmark results; maybe using smth like hyperfine will produce more stable figures). Is this the biggest remaining hot spot? If not, this could be premature optimization.
The same effect if not more can likely be achieved by optimizing pyenv-commands. The same algorithm that we ultimately used in pyenv-versions seems applicable here: https://unix.stackexchange.com/questions/798374/using-find-to-find-a-file-in-path
|
Revised per your feedback. The optimization now lives in
I tried the Thanks for the The timing improvement is marginal. But the code is cleaner either way: |
When called with --sh, glob only pyenv-sh-* files instead of all pyenv-* files. This skips iterating non-sh commands entirely. For all modes, replace the sort | uniq pipeline with sort -u (one fewer process) and iterate PATH directly with IFS=:; for path in $PATH instead of allocating an array.
jakelodwick
force-pushed
the
inline-sh-commands
branch
from
0f0ac35 to
f63a795
Compare
jakelodwick
changed the title
There was a problem hiding this comment.
I wouldn't call the code "cleaner" at all. Now we have lots of duplication: two very similar loops, two sort -u calls; overriding IFS for the rest of the script.
I've tried to radically change the algorithm from loops to a pipeline -- but that didn't save any more time than your version, either.
So I've eliminated the duplicate sort call and IFS override while keeping the separate loop.
If you're trying to optimize init, probably look into what other hot spots are remaining so as to make a real difference; maybe replacing eval "$(pyenv init)" with source <something> (like e.g. OhMyZsh does) will be faster. 'Cuz this PR's approach looks rather unproductive: we've just spent 2 days for no real gain. Also could check other performance-related issues that I've linked from the previous PR. E.g. the problem with Pyenv-Virtualenv's prompt command is real bad.
3 participants
pyenv-commands --shiterates allpyenv-*files in PATH, then filters for thesh-prefix. It pipes throughsortanduniqas separate processes.Three changes, all inside
pyenv-commands:--shfast path: globspyenv-sh-*directly instead of globbing allpyenv-*and filtering. Skips iterating non-sh commands entirely.sort -ureplacessort | uniq(one fewer process, all modes).IFS=:; for path in $PATHiterates PATH entries directly instead of allocating an array.Benchmark (
hyperfine --shell=none, 100 runs, Apple M2):The timing gain is small. The code is cleaner:
--shdoes less work, dedup is one process instead of two, PATH iteration has fewer intermediaries.All 234 tests pass, including bash 3.2 (
make test-unit-docker-3.2.57).