Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

10 second delay between zsh prompt displays after command completes(yes TEN) #964

Closed
fryjustinc opened this issue Aug 21, 2018 · 12 comments
Closed

Comments

@fryjustinc
Copy link

I didn't believe that powerlevel 9k could really be what is causing this but after individually commenting out every single line in my zshrc, totally replacing my zplug install, and switching my powerlevel9k config to default, the only thing that fixed the crazy lag in between prompts was commenting out powerlvl9k loading in my .zshrc (remember this is with DEFAULT config too). I posted a picture for your amusement at my suffering. (check out the timestamps on the right) I definitely could use some help disecting this one. (and no my home dir isn't tracked but I do have about 300 or so tracked projects throughout my drive, although they're not in path.
screen shot 2018-08-21 at 4 24 42 am

@truongnmt
Copy link

Yes, I'm suffering from the same behavior. Seem that it's so bloated.

@crsantos
Copy link

Same here, unfortunately.
Not sure if it's prezto's fault... but I started feeling this delay when I pulled 600 commits from powerlevel9k's master branch.
Happy to help debugging this.

@fryjustinc
Copy link
Author

@truongnmt @crsantos @bhilburn I got frustrated enough to dive deep. Turned on zprof and tracing and started debugging.

Here's what I found, there are two main culprites and a few suboptimal coding implementations that can be fine-tuned:

  1. The current context prompt is created by a [[sudo -n true]] test on every single rendering if you don't set a default user. sudo is a ridiculously slow command. I personally would instead do id -un since it is instant. also: set a default user. Seriously.

  2. the rbenv prompt takes a few seconds on its own to render. I hardly use ruby so I can't say with any certainty whether this is due to the implementation of the test for a ruby environment or ruby itself. I simply don't have the expertise to know what would factor into this on a first glance without looking stuff up

  3. Those two problems on their own being removed/fixed brought the render time down to about 1 second so the rest is just optimization. That being said, the current method for parsing color choices by the user is just about as inefficient as it is possibly to make it. Every single prompt segment on every single render compares the color the current segment against the previous one by iterating fully through a 200+ index array. Why not parse the color choices into ints on init and compare those? Or create an array of comparison values which are updated on a config change? etc. etc.

  4. there is a test if [[ $(command git rev-parse --is-inside-work-tree 2> /dev/null) == 'true' that seemed to run at kind of oddly random times, but generally around once every 3-4 renders. Git's own "git rev-parse --is-in-work-tree" is utilized here and, while it's probably not possible to get this incredibly quick due to the nature of testing all the parent directories, I'm tempted to implement something more specialized to eliminate some bloat. vcprompt is decent.

I'm going to do implementations of all of these changes and, if @bhilburn doesn't mind, submit them for a pull request.

Hope this helps you all

@bhilburn
Copy link
Member

bhilburn commented Sep 6, 2018

@fryjustinc - Thank you so much for taking the time to deep-dive on this and look at things further! I really do appreciate you investigating things and providing insight & suggestions :)

  1. Regarding sudo -n true, try pulling master and having another look. This behavior actually just changed with the merge of Use SUDO_COMMAND to check for sudo #937 a couple of days ago. That said, id -un might still be the better option? Setting a default user probably isn't a bad idea, either, actually - I'm surprised it's taken this long for someone to suggest that. Something as simple as p9kuser would go a long way to simplifying some conditionals.

  2. You're right - rbenv is a rather expensive command, and probably one that doesn't need to be in the default segment list.

  3. Yes, that color comparison is a sore point for p9k, for sure. It's unfortunately not as easy as just parsing them on init, because many segments change their colors depending on segment status. Can you think of a way to more efficiently do this for a dynamically colored prompt?

  4. I'm always loathe to bring in additional dependencies, although I would very seriously consider it if it would speed up the vcs segment; git, especially, can be horribly slow. I am a little wary about vcprompt, though, as it looks like it was abandoned 4 years ago. I will happily consider any suggestions you have in this space, though! Especially in large git repositories, the vcs segment can slow to crawl, and not withstanding asynchronous segment drawing (coming in v0.7.0), we haven't found a good way to speed it up.

PRs would be fantastic! If you want to submit PRs for the items in #1, that would be excellent! I'll handle #2 myself. What are your thoughts on #3 and #4? Also, would be good to get @dritter's and @onaforeignshore's thoughts, here.

Edit: Handled #2, now pushed to master.

@fryjustinc
Copy link
Author

@bhilburn ill put in a quick pull request for #1 later today. What I'm probably going to do is simply switch all the sudo -n true instances with id -un and an equality test against sudoit also would provide the username too so that would eliminate a later test. glad to hear 2 is taken care of. 3, I can definitely think of a few solutions but most of them would involve some kind of pseudo compilation step in between configuration and actually running the theme. There are data structures that would speed it up if you didn't want to do that though. 4, I would rewrite vcprompt myself. It's not that large. If you have the time, do you have a way I could reach out to you, or the other two individuals you mentioned if appropriate, to ask a few questions about functionality requirements and what exactly some of these steps doing (im thinking of the color stage). If not, that's no big deal either.

@bhilburn
Copy link
Member

bhilburn commented Sep 6, 2018

@fryjustinc - Yeah, compilation isn't really a good option for us unless it's auto-invoked by p9k and the dependencies are exceedingly simple. More efficient data structures would be excellent, though.

The codebase is about to undergo a titanic shift (i.e., a shift to a complete rewrite, that's already waiting & ready to go). Chatting on the phone or by VC would be great. Feel free to email me (bhilburn gmail) and we can go from there =)

@onaforeignshore
Copy link
Contributor

  1. Already done
  2. Already done
  3. Implemented in next
  4. After some research, it turns out this test is redundant so I am removing it in the VCS PR I am working on.

@dritter
Copy link
Member

dritter commented Nov 16, 2018

Hey @fryjustinc ,

I just wanted to drop by and ask you if you could check our next branch, cause we did some massive performance improvements there. Thx.

@fryjustinc
Copy link
Author

fryjustinc commented Nov 19, 2018 via email

@bhilburn
Copy link
Member

@fryjustinc - Thanks! Please let us know what you find with the VCS performance on next, and any ideas you have for further improving it!

@romkatv
Copy link

romkatv commented May 22, 2019

@fryjustinc If you are still using Powerlevel9k, could you check if performance issues can be reproduced with Powerlevel10k?

@bhilburn
Copy link
Member

I am on a issue-cleaning spree. This issue hasn't had motion in quite some time, and so I am closing it out due to inactivity. If anyone would like to re-open it because they feel it is un-resolved and is something they care about, please feel free to re-raise the discussion!

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

No branches or pull requests

7 participants