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

[zsh] Reload shared history before searching #2251

Merged
merged 4 commits into from
Dec 5, 2020
Merged

[zsh] Reload shared history before searching #2251

merged 4 commits into from
Dec 5, 2020

Conversation

mpolden
Copy link
Contributor

@mpolden mpolden commented Nov 14, 2020

When history sharing across shells is enabled (setopt SHARE_HISTORY), history written by shell A won't be available in shell B until re-rendering the prompt in B (e.g. by pressing Enter at the prompt).

With this change, C-r will always reload the history file if history sharing is enabled, making the latest history searchable immediately.

@mpolden
Copy link
Contributor Author

mpolden commented Dec 5, 2020

Last commit only reloads new history events, this prevents the event number of existing history entries from changing (which happens when you reload the entire history).

@junegunn junegunn merged commit b62a74b into junegunn:master Dec 5, 2020
@junegunn
Copy link
Owner

junegunn commented Dec 5, 2020

Merged, thanks!

@KimNorgaard
Copy link

This change has introduced a noticeable latency when using CTRL-R. On my system (arch/5.9.12 on lenovo t470s) this amounts to around 0.6 seconds:

time (fc -RI)
( fc -RI; )  0.59s user 0.00s system 99% cpu 0.595 total

I would suggest making this behavior optional since having to wait half a second for each CTRL-R is kind of draining :)

@junegunn
Copy link
Owner

junegunn commented Dec 8, 2020

@KimNorgaard Thanks for letting me know. I'm going to have to revert this for now since 0.59s is unacceptable.

junegunn added a commit that referenced this pull request Dec 8, 2020
@junegunn
Copy link
Owner

junegunn commented Dec 8, 2020

And it looks like we don't need an option for this as one can write a function that calls fzf-history-widget after fc -RI and bind it to ^R.

@mpolden
Copy link
Contributor Author

mpolden commented Dec 8, 2020

Reverting is fine, it's 0.071s for me though. @KimNorgaard Just curious, how big is your history? (fc -l 0 | wc -l)

@KimNorgaard
Copy link

KimNorgaard commented Dec 8, 2020

It's 10000 lines which is my HISTSIZE. Timing fc -l 0 is 0.12s.

@mpolden
Copy link
Contributor Author

mpolden commented Dec 8, 2020

Is this a spinning disk by any chance? Strange that it would be so slow. On my 3 year old MBP:

~$ time (fc -RI)
( fc -RI; )  0.04s user 0.03s system 96% cpu 0.068 total
~$ time (fc -l 0 | wc -l)
   23659
( fc -l 0 | wc -l; )  0.08s user 0.01s system 105% cpu 0.086 total
~$ zsh --version
zsh 5.7.1 (x86_64-apple-darwin19.0)

@KimNorgaard
Copy link

Nah it's an NVME. .zsh_history is 580K in size. I just ran strace --absolute-timestamps=precision:us -p <pid_of_zsh> and although this is obviously much slower due to overhead from running strace, it clearly shows that for me it's the seek syscalls on .zsh_history that takes up all the time. I have no obvious explanation but 'll investigate further.

@KimNorgaard
Copy link

Right so I found out if I experiment with HISTSIZE things move a lot faster. I have both HISTSIZE and SAVEHIST set to 10000 but I'm not sure yet why that would impact searching history. Anyway if I set HISTSIZE to a value a bit higher than the actual number of saved commands, e.g. 10010 then things run smoothly.

@mpolden
Copy link
Contributor Author

mpolden commented Dec 8, 2020

I'm guessing fc -RI has to shift your history when appending new events, because it ends up exceeding $HISTSIZE. Mine is 100 000, with only 23 000 entries so no shifting is required.

@KimNorgaard
Copy link

Yeah, I think you are right, however it's pretty normal to set your HISTSIZE and SAVEHIST to the same value in which case this is bound to happen sooner or later, right? As long as no shifting is going on everything will be fast, though.

@tvrg
Copy link

tvrg commented Dec 17, 2020

I noticed the same delay as @KimNorgaard and found this commit and the revert on master.
Not sure if it helps, but I noticed that the index number increases by 10000 ($HISTSIZE) on every invocation. It's immediately fixed when I run unsetopt sharehistory.

Peek 2020-12-17 10-28

@mpolden
Copy link
Contributor Author

mpolden commented Dec 17, 2020

Index remains unchanged for me. What other options do you have (setopt)?

@tvrg
Copy link

tvrg commented Jan 5, 2021

Sorry for the long delay. Here are my setopts:

alwaystoend
autocd
autopushd
completeinword
extendedhistory
noflowcontrol
histexpiredupsfirst
histignoredups
histignorespace
histverify
incappendhistory
interactive
interactivecomments
login
longlistjobs
nonomatch
promptsubst
pushdignoredups
pushdminus
sharehistory
shinstdin

It's probably the default ohmyzsh configuration. IIRC the problem only happened with multiple tmux panes. But even then I couldn't reproduce it reliably.

kralicky pushed a commit to kralicky/fzf that referenced this pull request Jun 23, 2021
kralicky pushed a commit to kralicky/fzf that referenced this pull request Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants