[Pkg-xen-devel] [PATCH] debian/scripts: Optimize scripts

[Elliott Mitchell]

Sometimes it really helps to be able to guess what is going on behind the
scenes to produce the right result.  As such below is a v2 of this patch.
I provided my guesses for the numbers, this time I've got actual data.
The appropriate command is:

strace -f -o trace.out -e trace=%process /usr/sbin/xl list

The system calls of interest are clone(), which is the Linux system call
behind fork(); and execve() which is the one which actually executes a
program.  On modern flavors of Unix (like Linux) fork() has become *much*
cheaper, memory pages will only be copied as they are modified.  Despite
the lower cost, clone() is an *expensive* system call, allocating a new
process table entry and extensively modifying page tables.  execve()
isn't nearly as expensive, but does hint at complexity.

With the version presently in git, strace reported 6 clone() calls and
7 execve() calls.  With the attached version, strace reported 0 clone()
calls and 2 execve() calls.  This suggests a substantial reduction in
overhead by using the attached.

You may dislike not using -e for the first part, but that was reviewed
carefully to ensure any failures would be handled correctly.

The only potential difference is if a hypervisor using identical
versioning and files were present (presented
/sys/hypervisor/version/major and /sys/hypervisor/version/minor which
matched a version of Xen which was installed), one might get an error out
of the Xen utility instead of the script (ie `xl` could complain instead
of the script).  Sharing both major and minor versions for a Debian
release seems unlikely and would likely diverge quickly.  As such I
consider the gains to *heavily* outweigh this potential downside.


>From 7a5d43be30657fdb70c18050dc3d9e941a564e46 Mon Sep 17 00:00:00 2001
From: Elliott Mitchell <ehem+debian at m5p.com>
Date: Sun, 20 Sep 2020 21:28:53 -0700
Subject: [PATCH] debian/scripts: Optimize scripts

Fewer fork()s and execve()s quickly add up to significant savings.  I'm
concerned Debian is slowly headed towards recreating SunOS^WSolaris
5.7^W2.7^W7 and the layers and layers of scripts which killed
performance.

As these runtime scripts are heavily used, avoid all uses of external
programs by them.

Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
---
 debian/scripts/xen-utils-wrapper | 43 +++++++++++++++++++++++++++++---
 debian/scripts/xen-version       |  6 +++--
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/debian/scripts/xen-utils-wrapper b/debian/scripts/xen-utils-wrapper
index 4d27a62d33..a38ccc25fb 100755
--- a/debian/scripts/xen-utils-wrapper
+++ b/debian/scripts/xen-utils-wrapper
@@ -1,6 +1,41 @@
-#!/bin/sh -e
+#!/bin/sh
 
-COMMAND="$(basename $0)"
-DIR=$(/usr/lib/xen-common/bin/xen-dir)
+# This portion is fast-path, avoid external programs, NOTE: NOT -e HERE!
+
+COMMAND="${0##*/}"
+
+DIR=/sys/hypervisor/version
+read major 2> /dev/null < $DIR/major
+read minor 2> /dev/null < $DIR/minor
+VERSION="$major.$minor"
+
+DIR="/usr/lib/xen-$VERSION/bin"
+
+exec "$DIR/$COMMAND" "$@"
+fail=$?
+
+
+# Certainly didn't work, now report failures, slower-path
+set -e
+
+error() {
+	err="$1"
+	shift
+	echo "ERROR: " "$@" >&2
+	exit $err
+}
+
+[ -e "/sys/hypervisor/type" ] || error 1 "Can't find hypervisor information in sysfs!"
+
+read type < /sys/hypervisor/type
+if [ "$type" != xen ]; then
+	[ -n "$type" ] || error 1 "Can't read hypervisor type from sysfs!"
+	error 1 "Hypervisor is not xen but '$type'!"
+fi
+
+if [ ! -d "/usr/lib/xen-$VERSION" ]; then
+	error 127 "Can't find version $VERSION of xen utils (maybe xen-utils-$VERSION was already removed before rebooting out of Xen $VERSION), bailing out!"
+fi
+
+error 127 "Failed to execute $COMMAND: return $fail"
 
-exec "$DIR/bin/$COMMAND" "$@"
diff --git a/debian/scripts/xen-version b/debian/scripts/xen-version
index 492070a43b..5c42ad7351 100755
--- a/debian/scripts/xen-version
+++ b/debian/scripts/xen-version
@@ -6,10 +6,12 @@ error() {
 }
 
 if [ -e "/sys/hypervisor/type" ]; then
-    type="$(cat /sys/hypervisor/type)"
+    read type < /sys/hypervisor/type
     if [ "$type" = xen ]; then
         DIR=/sys/hypervisor/version
-        VERSION="$(cat $DIR/major).$(cat $DIR/minor)"
+        read major < $DIR/major
+        read minor < $DIR/minor
+        VERSION="$major.$minor"
     elif [ -z "$type" ]; then
         error "Can't read hypervisor type from sysfs!"
     else
-- 
2.20.1



-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-debian-scripts-Optimize-scripts.patch
Type: text/x-diff
Size: 2708 bytes
Desc: not available
URL: <http://alioth-lists.debian.net/pipermail/pkg-xen-devel/attachments/20200922/adf18b6a/attachment-0001.patch>