Make startup subshell async to speed it up - #5148
Conversation
github-actions
Bot
added
needs triage
|
Oh man, after all the tons of work that went into fixing subshell initialization races, this sounds maximally dangerous, and it comes with no tests. I wish @egmontkob could look into this, because he did the earlier fixing, but I guess he wouldn't have time and interest. And I don't have time at the moment, unfortunately. |
Yeah, I was kind of wondering if my "little change" would be dangerous for everything else. I can think of some tests & try to fix the failing builds but if this step is so relevant for the whole project I understand this might wait...indefinitely...anyhow thank you! Edit: builds are failing because I forgot to commit the test folder (sorry), I'll double check them and push them in the next days. |
9c64721 to
89b0e9c
Compare
|
I force pushed because I also did a rebase. I reviewed the tests I had written and now I pushed them. I tried to copy the style from the other tests. |
|
so much "fun" ... please squash the commits, and write a proper commit message. elaborate on how the event dependencies change, which potential problems you have considered, etc. Questions raised in the long thread's on egmont's PRs may also be relevant targets. some "answers" may be best provided by test cases. |
…ll's startup files init_subshell() used to fork the shell and then wait for it to source its startup files and report its first working directory, all before mc's panels were even created. With a heavy zsh setup (oh-my-zsh and friends) this delays the first screen by up to several seconds, and is what remains of ticket MidnightCommander#4625 after the deadlock fixes. Now init_subshell() only forks the shell, injects the init string and registers the CWD pipe as a select channel. The rest of the handshake (reading the CWD, probing the persistent command buffer, forcing the initial cd) runs from that channel's callback once the shell is ready. The panels appear right away with the default prompt; the real one shows up when the handshake is done, through the existing load_prompt() channel on the pty. What changes in the event flow: - before: init_subshell() blocks in feed_subshell() -> panels are created -> main loop, where the pty channel reads prompts. - after: init_subshell() returns -> panels are created -> main loop, where the CWD pipe channel completes the handshake and removes itself. Until then the pty channel drains the shell's startup output, exactly like feed_subshell(QUIETLY) used to. What can happen while the handshake is still pending, and how it is handled: - Ctrl-O or running a command: invoke_subshell() completes the handshake synchronously first, i.e. it blocks like before, with the same 10 second limit. - the user changes directory in the panels: subshell_chdir() is ignored until the handshake is done. The forced initial cd targets the panel's directory instead of the shell's own one, so the shell catches up. - the shell dies (exec failure, "exit" in a startup file): sigchld_handler() removes both select channels and turns the subshell off, as the blocking code did after feed_subshell() failed. - the shell exits later and is restarted (invoke_subshell(), command.c, execute.c): init_subshell() resets subshell_initialized, so the new shell goes through the same handshake. - select channels are disabled only around do_executev() and toggle_subshell(), both of which go through invoke_subshell(), so the shell is never left in its self-SIGSTOP while nobody is listening. - the ordering of the persistent buffer probe vs. the first prompt (see ticket MidnightCommander#4625) is untouched: the handshake still runs as one piece, only later. tests/src/subshell exercises the handshake completion, the synchronous fallback, chdir and shell death before and after the handshake, against a fake pty and CWD pipe. Signed-off-by: Giovanni Alzetta <giovannialzetta@hotmail.it>
89b0e9c to
8f008cf
Compare
|
Hi! Sorry for taking long but the failing build lead me to finding a few real issues in the async window and the earlier subshell threads:
Tests now cover also the guards for handshake and navigation. I now squashed everything into a single commit and tried to write a detailed commit message, describing the even flow before and after all changes I introduced. The performance is the same as before: on my machine I go from about 1s loading time to 30ms. To be clear: working on this I realised how this "little change" interacts with a lot of different functionalities; I hope there are no side effects I am not seeing, but I might be wrong... |
3 participants
Proposed changes
Checklist
init_subshell()currently forks the user's shell and then blocks synchronously until it finishes sourcing its rc file. On my mac this is very slow and it contributes to #4625 and #4781.This PR skips that wait:
init_subshell()still forks the shell at start up but does so asyncronously.It registers the CWD-report pipe with mc's existing event-loop machinery used for
background-job progress and the steady-state prompt updates in
load_prompt(), making this a minor code change.The loading is protected: if the user tries to access the shell before the async loading is done they'll wait for it finish loading.
On My macOS I measured the change (I use oh-my-zsh and other init routines):
init_subshell()'s blocking window drops from ~850ms to ~10ms.git commit --amend -smake indent && make check)Notes
I am a long time mc user, but it has been a while since I contributed to an open source project like this: I made this change because the wait time is pushing me towards not using mc. Please let me know if there is anything I missed or I could do better. Thank you!