Bug#492086: partman: menus are very slow
On Wednesday 23 July 2008, Jérémy Bobbio wrote:
> On Wed, Jul 23, 2008 at 10:31:12AM -0700, John Reiser wrote:
> > The menus used by the partition manager during an install in
> > low-memory mode are very slow. Several seconds elapse between
> > <Enter> and the next menu. This is too long. The delay should be
> > 1/2 second or less, similar to the reponse for choosing timezone
> > (continent, country, zone) at the beginning of install.
>
> Various improvements could probably be done, but we need people willing
> to work on that. Those changes would need to wait until the release of
> Lenny though.
Actually, I think that using an environment variable for 'cat' would be
quite low risk. After all, we _know_ that all partman scripts are
executed under the main partman script (called from the postinst).
So, adding the following in partman-base/partman should ensure it can be
safely used anywhere else in partman:
cmdCAT=$(type -P cat 2>/dev/null) || \
cmdCAT=$(which cat 2>/dev/null) || exit 1
export cmdCAT
- 'which' is not available in busybox (main use case), but 'type -P'
produces same result and works in busybox and bash (but not dash)
- fallback to 'which' should ensure compatibility in other environments
(Ubuntu?), even if other shells are used
- immediately fails on error
- I've chosen cmdCAT instead of just CAT to make it extremely unlikely
that we'd conflict with existing envvars in any scripts/programs that
are called (even outside partman itself)
Attached an example patch for partman-base/lib/base.sh. Note that I did
not replace all uses of 'cat' but basically only where it is used to get
info from files in /var/lib/partman/devices. IMO it should be limited to
that usage (which needs to be documented).
Doing similar patches for other scripts that are known to be called often
(update.d, display.d, check.d, active-partition, some sourced lib
scripts) or take relatively long due to looping (commit.d, finish.d,
recipe calculation in partman-auto) should be trivial.
As long as partman-base is uploaded first there should be no problems with
transition.
Main question is how much we would gain in practice from making just this
change. It would really need to be significant to make it worth doing for
Lenny.
The other options John proposes IMO carry to much risk of
incompatibilities between environments or make the code to obscure.
Cheers,
FJP
Index: partman-base/partman
===================================================================
--- partman-base/partman (revision 54516)
+++ partman-base/partman (working copy)
@@ -3,6 +3,12 @@
. /lib/partman/lib/base.sh
. /lib/partman/lib/commit.sh
+# 'cat' is used a _lot_ in partman; avoid always looking up its path
+cmdCAT=$(type -P cat 2>/dev/null) || \
+ cmdCAT=$(which cat 2>/dev/null) || \
+ exit 1
+export cmdCAT
+
abort () {
if [ -f /var/run/parted_server.pid ]; then
stop_parted_server
Index: partman-base/lib/base.sh
===================================================================
--- partman-base/lib/base.sh (revision 54516)
+++ partman-base/lib/base.sh (working copy)
@@ -111,10 +111,10 @@
ask_user () {
local IFS dir template priority default choices plugin name option
dir="$1"; shift
- template=$(cat $dir/question)
- priority=$(cat $dir/priority)
+ template=$($cmdCAT $dir/question)
+ priority=$($cmdCAT $dir/priority)
if [ -f $dir/default_choice ]; then
- default=$(cat $dir/default_choice)
+ default=$($cmdCAT $dir/default_choice)
else
default=""
fi
@@ -154,7 +154,7 @@
while { read num id size type fs path name; [ "$id" ]; }; do
part=${dev}/$id
[ -f $part/view ] || continue
- printf "%s//%s\t %s\n" "$dev" "$id" $(cat $part/view)
+ printf "%s//%s\t %s\n" "$dev" "$id" $($cmdCAT $part/view)
done
restore_ifs
done | while read line; do
@@ -837,21 +837,21 @@
humandev_dasd_disk () {
dev=${1##*/}
- discipline=$(cat $1/discipline)
+ discipline=$($cmdCAT $1/discipline)
db_metaget partman/text/dasd_disk description
printf "$RET" "$dev" "$discipline"
}
humandev_dasd_partition () {
dev=${1##*/}
- discipline=$(cat $1/discipline)
+ discipline=$($cmdCAT $1/discipline)
db_metaget partman/text/dasd_partition description
printf "$RET" "$dev" "$discipline" "$part"
}
device_name () {
cd $1
- printf "%s - %s %s" "$(humandev $(cat device))" "$(longint2human $(cat size))" "$(cat model)"
+ printf "%s - %s %s" "$(humandev $($cmdCAT device))" "$(longint2human $($cmdCAT size))" "$($cmdCAT model)"
}
enable_swap () {
@@ -867,7 +867,7 @@
while { read_line num id size type fs path name; [ "$id" ]; }; do
[ $fs != free ] || continue
[ -f "$id/method" ] || continue
- method=$(cat $id/method)
+ method=$($cmdCAT $id/method)
if [ "$method" = swap ]; then
swaps="$swaps $path"
fi
@@ -888,7 +888,7 @@
local path device dev
dev="$1"
cd $dev
- device=$(cat device)
+ device=$($cmdCAT device)
grep "^$device" /proc/swaps \
| while read path x; do
swapoff $path
@@ -916,7 +916,7 @@
# First check if we should lock a device
if [ -e device ]; then
- testdev=$(mapdevfs $(cat device))
+ testdev=$(mapdevfs $($cmdCAT device))
if [ "$device" = "$testdev" ]; then
echo "$message" > locked
cd "$cwd"
@@ -950,7 +950,7 @@
# First check if we should unlock a device
if [ -e device ]; then
- testdev=$(mapdevfs $(cat device))
+ testdev=$(mapdevfs $($cmdCAT device))
if [ "$device" = "$testdev" ]; then
rm -f locked
cd "$cwd"
Reply to: