-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add prewarm option for additional ssh kitten connections #5159
Comments
How are you getting 100ms and 50ms that does not sound correct at all. time kitty +kitten ssh -h gives real 0.064 aka 64 ms for running main and executing ssh itself. I doubt my laptop Adding a return to line 702 so that ssh is not actually called but all Finally: real 0.132 And python 3.11 should reduce these times by about 20% |
Indeed it wasn't correct. Thanks for catching that. I was running with KITTY_DEVELOP_FROM set to let me change the Python code. If I don't have it set, I get more like 80 millis for a no-op ssh invocation:
I'm guessing the frozen stuff from bypy is making a big difference there? Without the KITTY_DEVELOP_FROM penalty, I still see a noticeable slowdown from the ssh kitten as opposed to direct ssh. Here's the kitten's timing, which is with the shared session established and is representative from several runs:
And here's direct ssh timing:
I'm doing things like popping a file fuzzy finder on the remote host from a kitty keybinding, so the slowness is particularly noticeable as it's in my editing flow. I'm still motivated to speed this up as a result, either via the prewarming or some other mechanism. |
Well, it is always going to be slower than plain ssh+controlmaster since You could possibly get rid of the 60ms python startup+import time by |
Hrmm, but if we kept an ssh connection waiting around, wouldn't that eliminate the entire 250ms? That's what makes it seem worthwhile to me. I agree that it's overkill for 60ms. |
On Thu, Jun 02, 2022 at 10:02:17AM -0700, Charlie Groves wrote:
Hrmm, but if we kept an ssh connection waiting around, wouldn't that eliminate the entire 250ms? That's what makes it seem worthwhile to me. I agree that it's overkill for 60ms.
You cant keep an ssh connection waiting around unless you are willing to
eliminate following cwd. The best you can do is have the kitten wait
just before building the tarball to send.
|
Thinking about it some more there is a prewarm I would be willing to add to kitty, but one that is generic for all kittens not specific to the ssh kitten. Basically, the idea would be to keep a background process that finishes all python initialization and imports most commonly used modules and then waits. When running kittens kitty will then pass the kitten envvars, cli, cwd and stdin to this waiting process and it will run the kitten by forking. This avoids the entire python initializationa nd import time overhead for all kittens. The slightly tricky part would be to successfully replace the stdio handles in the already initialized python. EDIT: Thankfully, os.dup2() works fine for this. If you want to help with this let me know. |
Sure, I'd be interested. Would you like me to take a swing given your general idea of swapping in a prewarmed python with dup2, or would you like to see more details before I throw some code together? |
No need I have already mostly implemented it in master. Feel free to |
prewarming for all kittens is now fully implemented in the prewarm branch. It works when runnign any kitten via keybindings or remote control. Note the very first time you run a kitten in a given kitty instance there wont be any benefit, after that prewarming will kick in. |
And I have now removed the limitation that the first kitten invocation is not prewarmed. It's now merged into master and will be in the next release. |
I piped That took 166 millis on average with the kitty nightly and 202 millis in the released version. 36 millis faster, so seems like the prewarming is working to me. Thanks for implementing it! Do I understand correctly that for my custom kittens, they'll use a prewarmed process if launched via shortcut or remote control but that it won't have their custom code imported? They seem to be working fine, just making sure that they're actually affected and worth testing as part of this. Anything else you'd like me to try? |
On Tue, Jun 14, 2022 at 02:25:13PM -0700, Charlie Groves wrote:
I piped `kitty --debug-keyboard --dump-commands` into a script that prepends each line with the process time in nanos. In the spawned kitty, I hit the "new window" keyboard shortcut in a window running the ssh kitten. I then grabbed the time for `handled as shortcut` being printed and the time for the final draw command for my prompt on the remote host.
That took 166 millis on average with the kitty nightly and 202 millis in the released version. 36 millis faster, so seems like the prewarming is working to me. Thanks for implementing it!
You're welcome, it was a fun technical challenge.
Do I understand correctly that for my custom kittens, they'll use a prewarmed process if launched via shortcut or remote control but that it won't have their custom code imported? They seem to be working fine, just making sure that they're actually affected and worth testing as part of this.
Yes, that's correct but importing one or two python modules is not slow,
should be sub millisecond.
Anything else you'd like me to try?
Well I am working on an implementation of prewarming that also works for
kittens launched at the shell inside kitty (think of icat or the diff
kitten). If that works out, I will post here.
|
The prewarm implementation for kittens at the shell is now in master. |
@kovidgoyal
How to fix this? Would you mind adding |
Should be fixed by: f637bf2 |
I noticed that the beginning of the remote control tutorial mentions ls | fzf | kitty @ send-text --stdin --match ... |
I doubt that would work without socket prewarming either. If it does it whatever | KITTY_PREWARM_SOCKET= kitty @ send-text |
And with this commit: 609d42e on my linux box at least even using fzf works, but I should emphasize, this is very much a hack, you cant really have multiple programs thinking they own the tty at the same time. |
Thanks for the explanation, these two programs will work simultaneously and will cause problems. For fzf, I guess the following will work. ls | fzf --bind 'ctrl-y:execute(printf {} | kitty @ send-text --match recent:1 --stdin)' |
@kovidgoyal
Also, due to recent changes,
|
That crash implies that the forking of the prewarm process is happening |
This should fix it: 5153d33 |
I compared Running the command |
On Sun, Aug 14, 2022 at 10:49:46PM -0700, page-down wrote:
> This should fix it: ...
I compared `show_kitty_env_vars` and forgot that `LANG` has been modified under macOS, just as `PATH` has been. That's why I couldn't find the difference, before digging into `main.py`.
Running the command `kitty/launcher/kitty +kitten/+runpy/...` in the development directory shows that the code executed is from the current running kitty terminal. The reason for this is perhaps not so obvious for developers unfamiliar with kitty. Is it necessary to document in the dev page that `KITTY_PREWARM_SOCKET` should be unset (set to empty)? Hopefully that will reduce the number of issues related to this.
You are welcome to send a PR for this. Though there isnt a dev page as
such. Maybe in CONTRIBUTING.md
|
Is your feature request related to a problem? Please describe.
It takes about 200 milliseconds to ssh into a host I'm using with the ssh kitten. That's the average value I get running
time kitty +kitten ssh jojr true
. I get an average of ~30 millis fromtime ssh jojr true
, which also has ControlMaster on. That additional 170 millis is a noticeable pause when I pop open a new window withlaunch --cwd=current
or similar commands.Describe the solution you'd like
When the ssh kitten finishes connecting, it launches another ssh kitten in the background. The next request for an ssh kitten to that host uses that already connected kitten. That should make it appear that connecting is instantaneous. Once that connection is in use, another background kitten could be launched to prewarm for the next request.
I'd think this should be behind an option and off by default. It would mean that any files or configuration that have changed since the previous invocation that would affect the kitty ssh bootstrap would be out of date, and I think users would want to be aware of that before enabling this. It would also need a way to update the cwd of the remote connection right before handing over the prewarmed connection since that will likely have changed since the bootstrap was run.
Describe alternatives you've considered
I did some profiling of the ssh kitten to see if I could speed it up enough to make it not an issue. I added time.monotonic calls to the start of execution in ssh/main.py and to right before ssh/main.py executes ssh. Those showed that it took 100 millis of Python execution to get to the start of ssh/main.py's code and another 50 millis to get to executing ssh. Since over half the additional time taken by the ssh kitten over raw ssh was in Python startup and importing, I didn't think I'd be able to optimize the ssh kitten enough to make a difference.
I've looked at using ssh directly myself, but that means reimplementing the cwd support of launch as far as I can tell.
Additional context
If this sounds like a reasonable thing to add, I'd be happy to start on a PR for it. I'm also happy to work on other approaches if there's a better way to go about making additional ssh connections fast.
The text was updated successfully, but these errors were encountered: