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

Suggestions slow since 6d25df6... #164

Closed
modk opened this issue Jun 3, 2016 · 7 comments
Closed

Suggestions slow since 6d25df6... #164

modk opened this issue Jun 3, 2016 · 7 comments

Comments

@modk
Copy link

modk commented Jun 3, 2016

Starting with 6d25df6 suggestions have become terribly slow for me. The commit switched from using fc -m $prefix to history[(R)$prefix*] which is much slower for me. Granted, my history is not tiny (>500 000 and growing) but zsh-autosuggestions have become close-to-unusable after the change for me. I notice a delay of up to one second per keypress. Reverting the change locally gets me back to usable performance.

I suppose fc's filtering is somehow optimized compared to doing history subscripting/filtering/globbing by hand?

@modk
Copy link
Author

modk commented Jun 3, 2016

Just saw that fc was introduced not so long ago. Probably for performance reasons? At first, I thought fc -ln -m $prefix was faster because it only queries the last 16 (?) entries by default but changing the command to fc -ln -m "$prefix*" 1 2>/dev/null | tail -1 i.e. starting at the first entry is equally fast for me. What does history[(R)$prefix*] do differently that it's needed here? Are there different escaping rules or cases where fc fails?

@ericfreese
Copy link
Member

Just saw that fc was introduced not so long ago. Probably for performance reasons?

It wasn't really introduced for any great reason. I had just learned about it and thought it might be a more straightforward way to fetch the suggestion. Given the problems it created (see #120) and the fact that I didn't fully understand it, I decided to revert it.

my history is not tiny (>500 000 and growing)

It probably doesn't make sense to search all 500,000 history entries. Maybe the fix here is to switch back to fc and create a configuration option for the number of entries to search back.

What does history[(R)$prefix*] do differently that it's needed here?

Basically, history[(R)$prefix*] was known to work, while fc wasn't.

If you have some time, would you mind double-checking that your fc command is truly searching your entire history, and then benchmarking the two approaches (fc vs. history)?

@modk
Copy link
Author

modk commented Jun 7, 2016

Did a quick benchmark:

time (repeat 10 {echo $history[${(k)history[(r)a*]}] > /dev/null})
1.02s user 0.06s system 99% cpu 1.089 total
time (repeat 10 {fc -ln -m "a*" 1]}] > /dev/null})
0.52s user 0.05s system 99% cpu 0.568 total

For reference:

time (repeat 10 {grep -i "a*" -m1 $HOME/.histfile > /dev/null})
0.00s user 0.04s system 97% cpu 0.036 total

On my machine, fc is twice as fast. On another machine with even larger history (> 1 000 000) fc was too slow for interactive use ($history... was even slower). Therefore it's probably useful to include a limit for searched history entries as proposed in #120. There might be problems with fc's pattern matching I'm not aware of but for my daily use, fc works, while history[... doesn't. If there is indeed a dropoff in performance at a certain number of history entries, I suppose it is an algorithmic issue then rather than an fc vs history[... one.

ericfreese added a commit that referenced this issue Jun 10, 2016
According to a few tests, the `fc` builtin appears to be quite a bit
faster than searching through the `$history` associative array when
dealing with large history files (500K+).
@ericfreese
Copy link
Member

Please take a look at the fixes/fc_builtin_for_searching_history branch and let me know what you think. It sped things up noticeably for me when using a large histfile.

I also added the -r flag to reverse the order of the results so that I could use head instead of tail, which should be a bit more efficient.

Therefore it's probably useful to include a limit for searched history entries

I didn't think of this earlier, but would HISTSIZE work for you? Or do you need all of those entries loaded but not searched by z-asug?

@ericfreese ericfreese mentioned this issue Jun 10, 2016
9 tasks
@modk
Copy link
Author

modk commented Jun 10, 2016

Looks good and is fast for me. Regarding HISTSIZE: I definitely want to keep all my history, I set the value that high on purpose as to keep all of it. For example when accessing the history via prefix search (history-beginning-search-backward-end, KEY_UP for me) after typing two or three letters it shrinks quite fast, probably leaving only some commands I haven't used in a while.

@ericfreese
Copy link
Member

ericfreese commented Jun 10, 2016

Looks good and is fast for me.

Ok cool, I've merged it into the develop branch and will release with v0.3.3.

I've also added a line-item to the v0.3.3 release to add a config option for a limit on the max number of history entries to search.

Thanks for your help with this!

@modk
Copy link
Author

modk commented Jun 11, 2016

Thanks! I appreciate your work on zsh-autosuggestions and like how the code has improved since. Hope we can get the full completion working (a la auto-fu, see e.g. #42 ) in the near future, I'll see what I can contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants