[PATCH] vars.sh: remove unnecessary fork to speedup boot/upgrades

[Trek]

On Tue, 1 Sep 2020 00:10:47 +0200 (CEST)
Thorsten Glaser <t.glaser at tarent.de> wrote:

> On Mon, 31 Aug 2020, Adam Borowski wrote:
> 
> > I don't think we should throw away a significant (10% w/o color /
> > 5% with) speed-up just because of a bug _somewhere else_.  
> 
> But I found something to possibly work around both this bug and
> the speed penalisation depending on shell implementation.

it seems this topic gatered your interest, so I would like to share
more details

previously my benchmarks were done with some unfinished patches applied
to lsb/init-functions, so I made again the benchmarks with only the
already sent patches applied to init-d-script (patch1-10) and
separately also to vars.sh (with the read -rN-1 variant too)

revision		dash	mksh	bash
-----------------------+-------+-------+-----
patch1-10		100.0	132.7	201.0
vars.sh-read		 93.3	125.6	175.6
vars.sh-read-rN-1	 93.3	125.0	175.7
fancy			328.3	413.4
buster			127.9	172.4	252.3
buster-fancy		355.9

on mksh using read it's faster than using $(cat), even if the builtin
is used, because it spawns a subshell (clone without execve), while
other shells need also to execute /bin/cat

using read -rN-1 it's even faster on mksh and the only caveat i could
imagine is that it would not work with ksh93, but it doesn't handle the
local keyword too, which is allowed inside Debian scripts

for cargocult people, may be we could teach them adding a comment like
this: never ever use this construct with other /proc files, see
https://bugzilla.kernel.org/show_bug.cgi?id=209031

fancy output is really slow, mostly because it forks many tput and some
echo, here the execve list when starting a service with the
debian/master version and dash

   # execve			source
----+--------------------------+---------------------------------
   1 /usr/bin/env		test-script (removed)
   1 /bin/sh			/lib/init/init-d-script
   1 /bin/run-parts		/lib/lsb/init-functions
   1 /bin/cat			/lib/init/vars.sh (removed)
   2 /usr/bin/basename		/lib/init/init-d-script (removed)
   1 /sbin/start-stop-daemon	/lib/init/init-d-script
   1 /bin/echo			/lib/lsb/init-functions (fancy)
  18 /usr/bin/tput		/lib/lsb/init-functions (fancy)

26 forks to start a daemon, 19 for the fancy output, 3 unavoidable
forks (sh, run-parts and start-stop-daemon) and 4 forks removed in
patches I sent

the benchmarks were made on an idle (physical) system:

  echo performance | \
    tee /sys/devices/system/cpu/cpu[0-9]*/cpufreq/scaling_governor

  export TESTSHELL=dash # mksh bash

  time sh -c 'for i in $(seq 1 10000); \
    do $TESTSHELL init-d-script test-script start; \
    done >/dev/null'

to test fancy output:

  time script -qc "sh -c 'for i in \$(seq 1 10000); \
    do \$TESTSHELL init-d-script test-script start; \
    done'" >/dev/null

where test-script is:

  DAEMON=/bin/true
  START_ARGS=--background
  NAME=findnothing
  PIDFILE=none

to gather the list of execve:

  strace -f script -qc "$TESTSHELL init-d-script test-script start" \
    2>&1 >/dev/null | sed -n 's/.*[e]xecve("\([^"]*\)".*/\1/p' | \
    sort | uniq -c

when time permits I will work on lsb patches, to speedup a bit the fancy
output, but may be it's not possible to do it and at the same time to
fix an old redirection bug or without using GNU tput extensions

many thanks for the review of the patches and please ask me if you want
more info or some change to be applied

ciao!