zsh-users / zsh-syntax-highlighting

Fish shell like syntax highlighting for Zsh.
github.com/zsh-users/zsh-syntax-highlighting
BSD 3-Clause "New" or "Revised" License
19.83k stars 1.33k forks source link

Cap highlighter execution time #361

Open psprint opened 7 years ago

psprint commented 7 years ago

I saw at friend's computer that zsyh caused ~0.8s delay after each key press when PWD was directory with 89000 files IIRC. Tested this on my machine and the lag is like 100 ms, though. However, what about NFS and cases where someone has even more files. I thought about a betterment: use float SECONDS to establish limit (300ms? zstyle?) on how long region_highlight should be generated. If the limit isn't kept then clear the table and stop generation of its entries. This would be triggered also for e.g. pasting long and/or sophisticated code.

danielshahaf commented 7 years ago

(OT: Congratulations on filing a perfect square.)

I think you raise two separate issues:

  1. Speed on large directories
  2. Enforcing a timeout

I think a timeout is a good idea. It's also pretty trivial to implement (declare SECONDS outside the loop and break inside it if it exceeds some threshold). The real question would be whether we take this opportunity to make the threshold configurable via zstyles as opposed to global parameters.

I also have another idea: we already cache the "previous" value of $BUFFER in $_ZSH_HIGHLIGHT_PRIOR_BUFFER. We could compute the largest common prefix of these variables and re-use the previous invocation's $region_highlight array up to the common point. In the common case of editing near the end of the buffer this should be a big win. For the case of editing a huge function, something more interesting is going on:

In the workfow fned ${function}\n <edit-command-line> :wq ^X^W, highlighting is not invoked even once. I'm not sure whether that's a bug or a feature, given that highlighting would be slow and isn't needed since the actual editing is done in $EDITOR...

This "common prefix" idea would break down if somebody has a zle widget that changes any state z-sy-h cares about (for example, echo foo bar<mywidget> where mywidget creates or deletes the file foo).

psprint commented 7 years ago

Fned doesn't call highlighting because of the disable of vared you once pointed me to? (if [[ $CONTEXT == (select|vared) ]]; in main highlighter). As for your proposition, it arose to me that a gradual processing of $BUFFER could be implemented? 300 ms after each keypress (BTW I thought that the earlier 300 ms I gave is rather too small value as general limit) and extending of region_highlight – might be interesting for non-vared situations. For vared and large functions I think the 300 ms should be used every e.g. 1 second, so sched had to be used, and complexity and robustness of all this is difficult to predict. However I think it would be nice to see how large function in vared gradually gains color, and that too could be configurable and disabled via zstyle (I think zstyle is nice to introduce, "base" config can still be in parameters).

danielshahaf commented 7 years ago

As for your proposition, it arose to me that a gradual processing of $BUFFER could be implemented? 300 ms after each keypress (BTW I thought that the earlier 300 ms I gave is rather too small value as general limit) and extending of region_highlight – might be interesting for non-vared situations.

I think it would be very annoying to have a 300ms delay after every keypress.

For vared and large functions I think the 300 ms should be used every e.g. 1 second, so sched had to be used, and complexity and robustness of all this is difficult to predict.

I don't understand the phrase "sched had to be used".

However I think it would be nice to see how large function in vared gradually gains color, and that too could be configurable and disabled via zstyle (I think zstyle is nice to introduce, "base" config can still be in parameters).

See #362 about zstyles.

psprint commented 7 years ago

I mean zsh/sched module. Used it with zsh-morpho, verified to some extend that it's robust (stuff like TRAPALARM can cause lot of mess)

danielshahaf commented 7 years ago

I mean zsh/sched module. Used it with zsh-morpho, verified to some extend that it's robust (stuff like TRAPALARM can cause lot of mess)

I see. As I said in my last comment, I believe a regular 300ms delay would result in bad UX.

danielshahaf commented 7 years ago

How often does this problem occurs? I'm trying to estimate how urgent/important this issue is.

psprint commented 7 years ago

I couldn't reproduce it, but for sure I saw it happening. Could someone give a report about NFS? If there is a problem then this should be treated as important, it's then like Cygwin – not a main platform, maybe not that common as Linux boxes (let's assume this for a moment), but failing at it wouldn't make ZSYH look good (thanks to m0viefreak's patches I gues that one is solved).

danielshahaf commented 7 years ago

I tried sshfs to a remote server, in a not-as-large directory. It was slower than locally, but usable. Given that I passed -o cache=yes to sshfs, I'm inclined to think the root cause of the slowness lies with sshfs or possibly the kernel (I'd expect performance when caching is enabled to be identical to performance on local directories).

So, summary so far:

  1. Anecdotal evidence of slowness on large directories. I'd rather see a reproducible case of this before we start thinking of way to fix it. Let's track this problem on a separate issue.
  2. Idea to cap highlighter execution time. Tracked as #361 (this issue).
  3. Idea to optimise through common prefixes. Tracked as #369.
bmhatfield commented 7 years ago

I have an additional idea that may help with this discussion.

For (1), it's easy for me to reproduce on sshfs where I get the bad UX of keystrokes not appearing until some time after I type them. EDIT: I realize this is not the same as large directories. Apologies if this confuses the point of (1).

For (2), instead of a cap on execution time, I recommend an async deferment where processing does not occur until I stop typing. I don't mean to imply 300ms of synchronous wait-for-each-keystroke, but a moving window that prevents highlighting from attempting to process until there's some very short break in typing. This would deal with laggy situations. A more advanced implementation could even automatically tune that delay to match the processing time of the syntax highlighter - if it takes 1ms to run, then you can run it every 1ms. If it takes 200ms, the window should be 200ms.

abread commented 6 years ago

A execution time cap would save me right now. Whenever I type ls /afs/ and some character (except space) after it, the terminal freezes before even showing the character written on the screen.

I disabled other plugins and customizations and managed to nail it down to this one.

The /afs folder is mounted by OpenAFS and is only connected to 3 servers (therefore 3 subfolders, I replaced the default enormous list). It's a slow filesystem/connector/thing and I can't make it any faster no matter how much I fiddle with the options.

danielshahaf commented 6 years ago

@addobandre Your best bet is probably to patch _zsh_highlight_main_highlighter_check_path so paths that start with /afs aren't stat()-ed. I'd be happy to take a patch that adds allows the user to configure a set of paths that shouldn't be highlighted; if you're interested could you please pursue in a new issue?