rupa / z

z - jump around
Do What The F*ck You Want To Public License
16.4k stars 1.17k forks source link

Check the running shell using environment variables #155

Closed mdirik closed 9 years ago

mdirik commented 9 years ago

I think checking if some shell specific environment variables are set is a nicer method than trying to execute shell builtins, so this commit changes the checks in z.sh file to use environment variables ZSH_VERSION and BASH_VERSION instead of executing compctl and complete.

Now I'll explain how trying to run shell builtins can be detrimental to performance:

I was lately observing some slow start while starting new shells. After profiling my .bash.rc, I found the culprit was this line in z.sh: if compctl >/dev/null 2>&1; then

Here is the relevant part from profile:

...
0.00066 1.84671  +++ compctl
0.00003 1.84675  +++ '[' -x /usr/lib/command-not-found ']'
1.23529 3.08204  +++ /usr/lib/command-not-found -- compctl
0.00131 3.08336  +++ return 127
...

It seems that z.sh tries to execute compctl, which doesn't exist in bash, so bash tries to run it as an executable file, which naturally fails. Then, since there is no such an executable in the system, command_not_found_handle kicks in and calls command-not-found, which blocks the shell startup for a significant time.

There are some more other shell specific environment variables but these two seems to fit the bill. If you have any other concerns I'm willing to improve it further.

Regards, Mert

jeanmichelcote commented 9 years ago

Thanks, it is indeed waaay faster to load a new shell session on bash. Couldn't this be merged?

rupa commented 9 years ago

Yeah it's a lot faster, I kind of want to do it, but I've been trained to rely on capabilities over variables in shell scripts, and i have some hesitation.

For example, one of the reasons the code's like this is that (IIRC) zsh can be configured to use bash compatible tab completion, so it's possible that a zsh user would have complete but not compctl. They can also have both available, so checking for complete first wasn't ideal. It's been a while since I've poked at this stuff, and I'm not a zsh user, but there were a few issues about it, and this setup seemed to make things "just work" for most people.

On the other hand, checking the variables will work most of the time, is conceptually simpler, and is indeed way faster.

rupa commented 9 years ago

Maybe something like if type compctl >/dev/null 2>&1; then would be faster

mdirik commented 9 years ago

@rupa Is it now better?

I didn't benchmark it but it still feels fast. In bash it works fine but I haven't tested it in zsh.

jeanmichelcote commented 9 years ago

Indeed, @rupa, your proposition feels as fast as the shell variables checking. On bash, at least. Thanks @mdirik and @rupa.

rupa commented 9 years ago

Yeah I tested it in zsh and on a couple different systems and seems cromulent, so pushed a change to use type. We'll see if bothers anyone :)

mdirik commented 9 years ago

That's great! Thanks all.

bean5 commented 9 years ago

Can this be closed?

mdirik commented 9 years ago

yep