[pve-devel] [PATCH installer] sys: command: wait for process exit with sub-second granularity

[pve-devel] [PATCH installer] sys: command: wait for process exit with sub-second granularity

[Christoph Heiss]

Using full seconds as a granularity for sleeping between waitpid()'s is
way too much and unnecessarily slows down the installation a lot. Most
processes take a few moments after closing their stdin/stdout to
actually exit fully, which means that we would sleep a second in most
cases.

Lower it to 0.1 second, which immensely improves the situation.

Some values for comparison; tui-installer on the same bog-standard
2-core, SeaBIOS, ext4, virtio VM (roughly averaged over multiple runs):

  * 8.0 ISO (baseline): ~2:30 min
  * w/o patch: ~9:00 min
  * w/  patch: ~2:30 min

Values measured are from pressing the 'Install' button until the
autoreboot dialog (aka. install finished) popped up.

Fixes: 152bbef ("sys: command: factor out kill() + waitpid() from run_command()")
Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Reported-by: Filip Schauer <f.schauer@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Proxmox/Sys/Command.pm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Proxmox/Sys/Command.pm b/Proxmox/Sys/Command.pm
index ac505c4..d2dcd42 100644
--- a/Proxmox/Sys/Command.pm
+++ b/Proxmox/Sys/Command.pm
@@ -9,6 +9,7 @@ use IPC::Open3;
 use IO::Select;
 use String::ShellQuote;
 use POSIX ":sys_wait_h";
+use Time::HiRes qw(usleep);
 
 use Proxmox::Install::ISOEnv;
 use Proxmox::Log;
@@ -52,12 +53,12 @@ my sub wait_for_process {
 
     kill('TERM', $pid) if $params{kill};
 
-    my $timeout = $params{timeout} // 5;
+    my $timeout = ($params{timeout} // 5) * 10; # waiting 0.1 secs per loop
     for (0 .. $timeout) {
 	my $terminated = waitpid($pid, WNOHANG);
 	return $? if $terminated > 0;
 
-	sleep(1) if $_ != $timeout; # all but last round
+	usleep(100_000) if $_ != $timeout; # sleep 0.1 sec, on all but last round
 	kill('KILL', $pid) if $params{kill} && $_ == 1; # just first round
     }
 
-- 
2.43.0

[pve-devel] applied: [PATCH installer] sys: command: wait for process exit with sub-second granularity

[Thomas Lamprecht]

Am 26/02/2024 um 17:50 schrieb Christoph Heiss:
> Using full seconds as a granularity for sleeping between waitpid()'s is
> way too much and unnecessarily slows down the installation a lot. Most
> processes take a few moments after closing their stdin/stdout to
> actually exit fully, which means that we would sleep a second in most
> cases.
> 
> Lower it to 0.1 second, which immensely improves the situation.
> 
> Some values for comparison; tui-installer on the same bog-standard
> 2-core, SeaBIOS, ext4, virtio VM (roughly averaged over multiple runs):
> 
>   * 8.0 ISO (baseline): ~2:30 min
>   * w/o patch: ~9:00 min
>   * w/  patch: ~2:30 min
> 
> Values measured are from pressing the 'Install' button until the
> autoreboot dialog (aka. install finished) popped up.
> 
> Fixes: 152bbef ("sys: command: factor out kill() + waitpid() from run_command()")
> Reported-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> Reported-by: Filip Schauer <f.schauer@proxmox.com>
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  Proxmox/Sys/Command.pm | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
>

applied this already yesterday, thanks!