4625 slow zsh macos #4934
4625 slow zsh macos #4934
Conversation
github-actions
bot
added
needs triage
egmontkob
added
area: core
ossilator
left a comment
ossilator
left a comment
There was a problem hiding this comment.
good enough for starters.
it's entirely plausible that there is no right solution for this; after all, mc is operating outside anything normally expected for interactive shells.
egmontkob
force-pushed
the
4625_slow_zsh_macos
branch
from
9b5b12a to
8a07ca6
Compare
egmontkob
force-pushed
the
4625_slow_zsh_macos
branch
from
8a07ca6 to
e260a46
Compare
…king mode This prevents a possible deadlock if the slave flushes the line just before we attempt to read from it. Signed-off-by: Egmont Koblinger <egmont@gmail.com>
Previously we omitted injecting a cd command to the subshell at startup if it was already in the target directory. The first prompt of the subshell was read and displayed in mc's panels. This plays badly with the forthcomming change where testing the persistent buffer code will read and discard the subshell's data to avoid a deadlock. The proper solution would be more complicated than justified. Just get a new prompt printed and we'll catch that. Signed-off-by: Egmont Koblinger <egmont@gmail.com>
… 1/2 Fix data loss while injecting commands to our subshell. This data loss affects all shells, not just zsh, and all platforms, not just macOS. Discarding the keypresses, i.e. removing the ones already placed in the tty line, either explicitly (tcflush()) or implicitly (by sending the interrupt character Ctrl+C) is useful when mc's panels are presented after running a command that the user initiated. Here we don't want letters that the user might have typed ahead to be sent to the subshell. However, when injecting our shell initialization code, these actions are harmful as they can cause parts of these commands to get lost. Signed-off-by: Egmont Koblinger <egmont@gmail.com>
… 2/2 Avoid a deadlock arising by zsh draining its output channel (i.e. waiting for mc to read them), while mc is waiting over a different channel for zsh's response to some magic keypresses to test the persistent command line feature. Fix this by making mc behave as it's expected from a terminal master: still consuming incoming data while performing that other task. The bug did not affect Linux because there draining the channel, at least over local pseudoterminals, is a no-op, it does not actually wait for the master side to read that data. Signed-off-by: Egmont Koblinger <egmont@gmail.com>
…ndings in our shell init strings Send one logical line to the shell so that it only executes the pre-prompt function and only prints the prompt once; however, split it into short physical lines for systems where cooked mode's buffer is small. Signed-off-by: Egmont Koblinger <egmont@gmail.com>
egmontkob
force-pushed
the
4625_slow_zsh_macos
branch
from
e260a46 to
9180349
Compare
zyv
left a comment
zyv
left a comment
There was a problem hiding this comment.
I'm slowly working through my immense mail backlog and have now made it to this PR... I've looked at all the commits separately and at the code as a whole once again.
To me it looks great now! I can particularly commend @egmontkob for splitting changes into sensible commits with helpful commit messages, and the comments in the code are also really useful.
Not even speaking of all the research and investigation that went into getting to the bottom of it. This is really a remarkable tour de force!
In as far as I'm concerned, I'd like to see it merged now. Okay with you @mc-worker ?
|
Let's see how it goes... I'm merging it now, and will give it another round of testing on various OSes in a couple of days (please remind me :)) |
|
Just for the record: A small startup delay on Solaris, #4812, still isn't fixed. There we'd need to somehow work around an obvious Solaris bug. |
My zsh startup delay on macOS is gone ❤️ |
4 participants
Proposed changes
Fix slow startup on macOS with zsh, and other related problems also affecting other platforms and shells.
Checklist
👉 Our coding style can be found here: https://midnight-commander.org/coding-style/ 👈
git commit --amend -smake indent && make check)This is a reincarnation of closed PR #4784. The patches are essentially unchanged (rebased and a few minor feedback addressed).
I really wanted to address #4253 / #4789 first, i.e. to inject our commands via shell startup files rather than "typing" them to those shells; and rebase this work on top of that. However, as discussed in 4789, I failed to figure out how to do this in
zsh, it looks like it's not possible. This made me lose motivation and I've abandoned that work, at least for now.The work in this PR is quite hackish, but so is the problem we're trying to deal with, and the current quite complex codebase. And, unfortunately, I still can't claim I perfectly understand everything, including the suspicious 2 or 3 different ways of handling of sending the Ctrl-C char. It's surely nowhere near the cleanest of my works I've ever made, but honestly I got tired of not seeing a clear solution. It's just not the kind of area I'm comfortable with.
So, IMO, let's land this -- along with remotely related pending PR #4804 --, both have already gotten approval a long time ago but then still the discussion and possible improvements went on. I'm afraid this very PR is not really going to get substantially better, and lack of progress for more than a month clearly shows that I've lost motivation. I suggest that we land these two after a last round of review, and then later (also depending on possible user feedback) iterate further.