-
Notifications
You must be signed in to change notification settings - Fork 220
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
Reduce toolbox enter
start time
#1070
Comments
Yes, it will be good to optimize |
Yes, you are right. We do spawn
You might be onto something. Looking at the lines that you pointed out:
The first one is an invocation of Then there are two instances of |
I am happy to report that I found a way to remove one of those |
Currently, the 'enter' and 'run' commands always invoke 'podman start' even if the Toolbx container's entry point is already running. There's no need for that. The commands already invoke 'podman inspect' to find out if the org.freedesktop.Flatpak.SessionHelper D-Bus service needs to be started. Thus, they already have what is needed to find out if the container is stopped and 'podman start' is necessary before it can be used with 'podman exec', or if it's already running. The unconditional 'podman start' invocation was followed by a second 'podman inspect' invocation to find out if the 'podman start' managed to start the container's entry point. There's no need for this second 'podman inspect' either, just like the 'podman start', when it's already known from the first 'podman inspect' that the container is running. The extra 'podman start' and 'podman inspect' invocations are sufficiently expensive to add a noticeable overhead to the 'enter' and 'run' commands. It's common to use a container that's already running, just like having multiple terminals with the same working directory, and terminal emulation applications like Ptyxis try to make it easier [1]. Therefore, it's worth optimizing this code path. [1] https://gitlab.gnome.org/chergert/ptyxis https://flathub.org/apps/app.devsuite.Ptyxis containers#1070
Currently, the 'enter' and 'run' commands always invoke 'podman start' even if the Toolbx container's entry point is already running. There's no need for that. The commands already invoke 'podman inspect' to find out if the org.freedesktop.Flatpak.SessionHelper D-Bus service needs to be started. Thus, they already have what is needed to find out if the container is stopped and 'podman start' is necessary before it can be used with 'podman exec', or if it's already running. The unconditional 'podman start' invocation was followed by a second 'podman inspect' invocation to find out if the 'podman start' managed to start the container's entry point. There's no need for this second 'podman inspect' either, just like the 'podman start', when it's already known from the first 'podman inspect' that the container is running. The extra 'podman start' and 'podman inspect' invocations are sufficiently expensive to add a noticeable overhead to the 'enter' and 'run' commands. It's common to use a container that's already running, just like having multiple terminals within the same working directory, and terminal emulation applications like Ptyxis try to make it easier to do so [1]. Therefore, it's worth optimizing this code path. [1] https://gitlab.gnome.org/chergert/ptyxis https://flathub.org/apps/app.devsuite.Ptyxis containers#1070
Done: #1491 The next step is to explore if we can get rid of the |
Currently, the 'enter' and 'run' commands always invoke 'podman start' even if the Toolbx container's entry point is already running. There's no need for that. The commands already invoke 'podman inspect' to find out if the org.freedesktop.Flatpak.SessionHelper D-Bus service needs to be started. Thus, they already have what is needed to find out if the container is stopped and 'podman start' is necessary before it can be used with 'podman exec', or if it's already running. The unconditional 'podman start' invocation was followed by a second 'podman inspect' invocation to find out if the 'podman start' managed to start the container's entry point. There's no need for this second 'podman inspect' either, just like the 'podman start', when it's already known from the first 'podman inspect' that the container is running. The extra 'podman start' and 'podman inspect' invocations are sufficiently expensive to add a noticeable overhead to the 'enter' and 'run' commands. It's common to use a container that's already running, just like having multiple terminals within the same working directory, and terminal emulation applications like Ptyxis try to make it easier to do so [1]. Therefore, it's worth optimizing this code path. [1] https://gitlab.gnome.org/chergert/ptyxis https://flathub.org/apps/app.devsuite.Ptyxis containers#1070
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable overhead to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBOX_RUN_USE_POLLING environment variable for testing. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
One thing that I failed to notice before is the possibility to make things faster when a container is used (ie., started) for the first time with We currently poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation, where it was relatively easier to poll than to use These days, with the Go implementation, we should use See: #1495 |
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable delay to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, which is quite easy to do with the Go implementation, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBOX_RUN_USE_POLLING environment variable for testing. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable delay to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, which is quite easy to do with the Go implementation, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBX_RUN_USE_POLLING environment variable for testing. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable delay to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, which is quite easy to do with the Go implementation, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBX_RUN_USE_POLLING environment variable for testing. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable delay to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, which is quite easy to do with the Go implementation, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBX_RUN_USE_POLLING environment variable for testing. Setting this environment variable disables some code to ensure that the polling ticker is actually used, because, otherwise, the race between the creation and detection of the initialization stamp file makes it difficult to test the fallback. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable delay to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, which is quite easy to do with the Go implementation, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBX_RUN_USE_POLLING environment variable for testing. Setting this environment variable disables some code to ensure that the polling ticker is actually used, because, otherwise, the race between the creation and detection of the initialization stamp file makes it difficult to test the fallback. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
Currently, the 'enter' and 'run' commands poll at one second intervals to check if the Toolbx container's entry point has created the initialization stamp file to indicate that the container has been initialized. This came from the POSIX shell implementation [1], where it was relatively easier to poll than to use inotify(7) to monitor the file system. The problem with polling is that the interval is always going to be either too short and waste resources or too long and cause delays. The current one second interval is sufficiently long to add a noticeable delay to the 'enter' and 'run' commands. It will be better to use inotify(7) to monitor the file system, which is quite easy to do with the Go implementation, so that the commands can proceed as soon as the initialization stamp file is available, instead of waiting for the polling interval to pass. There's a fallback to polling, as before, when the operating system is suffering from a shortage of resources needed for inotify(7). This code path can be forced through the TOOLBX_RUN_USE_POLLING environment variable for testing. Setting this environment variable disables some code to ensure that the polling ticker is actually used, because, otherwise, the race between the creation and detection of the initialization stamp file makes it difficult to test the fallback. [1] Commit d3e0f3d containers@d3e0f3df06d3f5ac containers#305 containers#1070
This is still left to be done or explored. However, I am inclined to close this issue for the time being because we did land the optimizations in #1491 and #1495 We can revisit this if people still find the |
Is your feature request related to a problem? Please describe.
I'm new to using
toolbox
, buttoolbox enter
seems to be a bit sluggish to launch(~1s). While still fast (I might just be nit-picking), my workflow is predominantly on the command line and so I'm constantly opening new terminal sessions, where the 1s is a significant pause before the prompt is visible.Comparing the run times between
toolbox run ...
andpodman exec ...
makes me think the problem is similar to that of #654. Might be related totoolbox
evaluating expensive paths to ensure safe fallback (speculation).Describe the solution you'd like
Either a way to bypass the fallback sanity/safety checks with a flag, or perhaps narrow the gap between
podman exec
andtoolbox enter
run times.Describe alternatives you've considered
Directly running
podman exec ...
instead oftoolbox enter
, on the launch of new terminal sessions.Some reduction can be achieved by specifying the container:
Additional context
I've tried to mark (
>>
) where the potential slow down might occurThe text was updated successfully, but these errors were encountered: