search

romkatv / zsh-bench

Benchmark for interactive Zsh
MIT License
624 stars 27 forks source link

zsh-bench runs indefinitely long #27

Closed Magniquick closed 9 months ago

Magniquick commented 9 months ago
Screenshot 2024-02-10 at 5 22 41 PM

My .zshrc is incredibly nonstandard, so I'll try to trim it down and see what causes the issue. on that note, how much time should it take for it to finish running / output something else ?

romkatv commented 9 months ago

how much time should it take for it to finish running / output something else ?

On my machine ~/zsh-bench/zsh-bench --iters 1 takes 13s.

romkatv commented 9 months ago

By the way, if your username contains sensitive information, consider changing it. Then you won't have to censor it.

Magniquick commented 9 months ago

By the way, if your username contains sensitive information, consider changing it. Then you won't have to censor it.

too lazy right now : ) Thanks for the advise - maybe later

Magniquick commented 9 months ago

seems like these lines cause the issue:

if $ZDOTDIR/plugins/iterm2/utilities/it2check; then
  is_iterm=True
fi

(I use that var to enable iterm2 specific things) it's crucial to note that it is that script that causes issues - merely setting the variable causes no issues Here is the file in question: it2check.txt That comes from here - the official iterm2 integration repo. (I'd debug this myself, but I'm not good enough at shell scripting or how terminals work to do so : D )

romkatv commented 9 months ago

I can see how that script can break things. It doesn't just break zsh-bench, it breaks your shell, too. If you start zsh and start typing, that script may read stuff from the TTY it wasn't supposed to. To add insult to injury, this script is very slow.

Consider removing that check and simply doing stuff unconditionally.

Magniquick commented 9 months ago

I can see how that script can break things. It doesn't just break zsh-bench, it breaks your shell, too. If you start zsh and start typing, that script may read stuff from the TTY it wasn't supposed to. To add insult to injury, this script is very slow.

Consider removing that check and simply doing stuff unconditionally.

ouch, thanks. Should I file a bug ? would this even be considered as one - as this 'breaking stuff' seems to be part of its function ?

(BTW, thanks for your work in the zsh field - I am currently using one of your diy configs as a base and it works extremely well for me !)

romkatv commented 9 months ago

Sure, You can file a bug, but I'm not sure how useful that will be. Basically, apart from being slow, this script has these issues:

  1. It consumes pending input. If you start typing while this script is running (while for you means typing while zsh is initializing), a portion of that input may silently disappear.
  2. It assumes that no input comes between dropping all buffered input and reading the response to the DSR. It's a race condition.

In general, reading TTY input and dropping it on the floor is a bad idea.

It is possible to implement this script correctly and make it fast, at least in zsh, but it's non-trivial.

romkatv commented 9 months ago

I am currently using one of your diy configs as a base

Just to clarify: the existence of this config in my repo does not mean that I endorse it. If you like it, by all means.

Magniquick commented 9 months ago

Sure, You can file a bug, but I'm not sure how useful that will be. Basically, apart from being slow, this script has these issues:

  1. It consumes pending input. If you start typing while this script is running (while for you means typing while zsh is initializing), a portion of that input may silently disappear.

  2. It assumes that no input comes between dropping all buffered input and reading the response to the DSR. It's a race condition.

In general, reading TTY input and dropping it on the floor is a bad idea.

It is possible to implement this script correctly and make it fast, at least in zsh, but it's non-trivial.

guess I'll more back to reading the env variables.

I am currently using one of your diy configs as a base

Just to clarify: the existence of this config in my repo does not mean that I endorse it. If you like it, by all means.

ofc - but it does work well for my workflow - and I dont see any large drawbacks. Thanks again !