Skip to content
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

high max-errors causes prolonged max-cpu consumption #946

Closed
catharsis opened this issue Aug 5, 2015 · 10 comments · Fixed by #953
Closed

high max-errors causes prolonged max-cpu consumption #946

catharsis opened this issue Aug 5, 2015 · 10 comments · Fixed by #953
Labels

Comments

@catharsis
Copy link
Contributor

So, I don't know if the solution here is "so don't do that", but I figured it might be worth bringing it up;

https://github.com/sorin-ionescu/prezto/blob/master/modules/completion/init.zsh#L70 sets max-errors to the length of PREFIX+SUFFIX/3. Now I'm completely new to this, but on my system, this means that doing something like

ls aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab>

when no file with a similar name exists in the current directory causes zsh to hang and peak on CPU% for a long time. The time needed to complete seems to grow close-to-exponentially. I know this example is pretty contrived but I think it demonstrates the issue well enough.

So, my query is: is there a better, perhaps more conservative way to set max-errors to avoid this hanging? It seems to me that deciding on a ceiling for max-errors, based on some clever heuristics might be worthwhile.

@el1t
Copy link

el1t commented Aug 18, 2015

I agree, the errors should definitely be capped. Most commands shouldn't have more than 10 errors anyways. For me, something like

zstyle -e ':completion:*:approximate:*' max-errors 'reply=($((($#PREFIX+$#SUFFIX)/3>7?7:($#PREFIX+$#SUFFIX)/3))numeric)'

capped it at 7.

catharsis pushed a commit to catharsis/prezto that referenced this issue Aug 20, 2015
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
@catharsis
Copy link
Contributor Author

@el1t That works much better; I took the liberty of submitting a PR with your change.

@el1t
Copy link

el1t commented Aug 20, 2015

@catharsis cool glad I could help!

@sorin-ionescu
Copy link
Owner

Can you show me an example of when you got high CPU usage? I'd like to test it myself.

@catharsis
Copy link
Contributor Author

@sorin-ionescu As described in the original report; Simply doing ls aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab> in a shell without the patch from #953 applied would cause zsh to consume 100% CPU on one core for an indeterminate time (I never cared to wait it out) trying to complete the completely wrong path. Shorter paths (smaller search-space) result in shorter time.

Of course, the same effect is seen for any path completions that aren't close to anything that exists in the file system.

@sorin-ionescu
Copy link
Owner

Don't type so many a then.

Give me a real usage scenario. I'm asking because I have had no trouble with the current maximum errors.

@catharsis
Copy link
Contributor Author

Ok, I see what you're saying, my mistake.

Well, consider for example the case where you're the kind of person who copies instructions from the internet when you're trying to fix a problem. The instructions say you should edit /usr/local/etc/fooserviced/config/init.cfg, so you copy the path and press tab to make sure you copied the entire thing correctly, but "oh no!" your corresponding config file is not in /usr/local at all, and now your shell is no longer responding: sucks to be you.

Or, perhaps you have a temporary device or file system at ~/somedevice and you have a command in your history to cat a file on it (or whatever). So you recall the command by doing ^R, get cat ~/somedevice/my_cat_pictures/2001-08-12/cute_kittens.png and tab just out of habit. But, too bad, you forgot you unmounted your device, because your spouse desperately needed some cat pics right now, and now you're stuck with an unresponsive shell and no kittens.

@catharsis
Copy link
Contributor Author

Here's another example that I just ran into:

I had a git commit that I needed to cherry-pick from a branch in my clipboard. I started typing:
git cherry<paste> My command line now looks like git cherry0e454967543e53624cceebd3fa4ab11260a9b024. I put my cursor at the end of cherry, and tab, thinking it'd result in git cherry-pick 0e454967543e53624cceebd3fa4ab11260a9b024. Instead, my shell becomes unresponsive and I have to kill it.

@catharsis
Copy link
Contributor Author

@sorin-ionescu Do you agree that this is a valid issue? Is there anything blocking #953 from getting merged?

@sorin-ionescu
Copy link
Owner

It is a valid issue. Nothing is blocking merging it. I will merge it.

toland pushed a commit to toland/prezto that referenced this issue Dec 30, 2015
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
belak pushed a commit that referenced this issue Apr 13, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes #946.
tenebrousedge pushed a commit to tenebrousedge/prezto that referenced this issue Apr 20, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
onshi pushed a commit to onshi/prezto that referenced this issue May 5, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
drpebcak pushed a commit to drpebcak/prezto that referenced this issue May 15, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
sgtsquiggs pushed a commit to sgtsquiggs/prezto that referenced this issue May 16, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
jdsutherland pushed a commit to jdsutherland/prezto that referenced this issue May 19, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
esebastian pushed a commit to esebastian/prezto that referenced this issue May 23, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
Raimondi pushed a commit to Raimondi/prezto that referenced this issue Jun 20, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
eduardolundgren pushed a commit to eduardolundgren/prezto that referenced this issue Jul 8, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
henrydobson pushed a commit to henrydobson/prezto that referenced this issue Jul 26, 2017
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
pfista pushed a commit to pfista/prezto that referenced this issue Nov 21, 2018
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
josh-h pushed a commit to josh-h/prezto that referenced this issue Dec 17, 2018
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
lalonde pushed a commit to lalonde/prezto that referenced this issue Feb 28, 2019
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
jgosmann pushed a commit to jgosmann/prezto that referenced this issue Nov 1, 2019
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
rooney pushed a commit to rooney/prezto that referenced this issue Aug 19, 2020
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
RIT80 pushed a commit to RIT80/prezto that referenced this issue Jan 25, 2022
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
RIT80 added a commit to RIT80/prezto that referenced this issue Jan 25, 2022
7 is pretty arbitrarily chosen, but seems like a reasonable tradeoff, at
least the completion no longer shows symptoms of exponential
time-growth when trying to complete something completely wrong.

This fixes sorin-ionescu#946.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants