Dave Miner wrote:
> J?rgen Keil wrote:
> > "6626043 create_ramdisk could be faster" needs a sponsor;
> > my suggested performance enhancements can be found in the
> > workaround section of the bug:
> > http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6626043
> >
> > My contributor agreement # is OS0003.
> 
> I'll sponsor this fix.

Erm... are you interested in a cleanup patch, too ? I just did a quick
look at the script and saw some stuff which could be done better+faster
(most stuff is based on
http://www.opensolaris.org/os/project/shell/shellstyle/), e.g. (the list
is incomplete)
---- snip ----
[snip]
     28 format=ufs
     29 ALT_ROOT=
     30 compress=yes
     31 SPLIT=unknown
     32 ERROR=0
     33 dirsize32=0
     34 dirsize64=0

Erm... typed variables may be nice (e.g. "integer dirsize=0" etc.) ...

     35 
     36 BOOT_ARCHIVE=platform/i86pc/boot_archive
     37 BOOT_ARCHIVE_64=platform/i86pc/amd64/boot_archive
     38 
     39 export PATH=$PATH:/usr/sbin:/usr/bin:/sbin

AFAIK this is completely wrong. If $PATH gets added then it should be
after all the primary stuff, otherwise adding something "unexpected"
which supersets normal tools in /usr/bin/ may cause havoc...

BTW: The script needs to reset LC_NUMERIC to "C" to avoid that a locale
with a non-'.' decinal delimter blows the script up. AFAIK this sequence
should work:
-- snip --
if [[ "${LC_ALL}" != "" ]] ; then
    export \
        LC_MONETARY="${LC_ALL}" \
        LC_MESSAGES="${LC_ALL}" \
        LC_COLLATE="${LC_ALL}" \
        LC_CTYPE="${LC_ALL}"
        unset LC_ALL
fi
export LC_NUMERIC=C
-- snip --

     40 
     41 #
     42 # Parse options
     43 #
     44 while [ "$1" != "" ]

Please use [[ ]] instead of [ ] ... or maybe even better ks93's version
of "getopts" ...

     45 do
     46         case $1 in
     47         -R)     shift
     48                 ALT_ROOT="$1"
     49                 if [ "$ALT_ROOT" != "/" ]; then
     50                         echo "Creating ram disk for $ALT_ROOT"
     51                 fi
     52                 ;;
     53         -n|--nocompress) compress=no
     54                 ;;
     55         *)      echo Usage: ${0##*/}: [-R \<root\>]
[--nocompress]
     56                 exit
     57                 ;;
     58         esac
     59         shift
     60 done
     61 
     62 if [ -x /usr/bin/mkisofs -o -x /tmp/bfubin/mkisofs ] ; then

[[ -x /usr/bin/mkisofs || -x /tmp/bfubin/mkisofs ]] may be better...

     63         format=isofs
     64 fi
     65 
     66 #
     67 # mkisofs on s8 doesn't support functionality used by GRUB boot.
     68 # Use ufs format for boot archive instead.
     69 #
     70 release=`uname -r`

Please use POSIX syntax, e.g. release="$(uname -r")

     71 if [ "$release" = "5.8" ]; then
     72         format=ufs
     73 fi
     74 
     75 shift `expr $OPTIND - 1`

Please use builtin math, e.g. shift $((OPTIND - 1))

     76 
     77 if [ $# -eq 1 ]; then

Please use arthmetric expressions for ksh scripts, e.g.
-- snip --
if (( $# == 1 )); then
-- snip --

     78         ALT_ROOT="$1"
     79         echo "Creating ram disk for $ALT_ROOT"

Please use "print", not "echo" (or maybe...
-- snip --
printf "Creating ram disk for %s\n" "$ALT_ROOT"
-- snip --
... is better...

     80 fi
     81 
     82 rundir=`dirname $0`

Please use POSIX shell syntax, e.g. 
-- snip --
rundir="$(dirname "$0")"


[snip - endless bickering removed]

    109 function getsize
    110 {
    111         # Estimate image size and add 10% overhead for ufs stuff.
    112         # Note, we can't use du here in case we're on a filesystem,
e.g. zfs,
    113         # in which the disk usage is less than the sum of the file
sizes.
    114         # The nawk code 
    115         #
    116         #       {t += ($5 % 1024) ? (int($5 / 1024) + 1) * 1024 : $5}
    117         #
    118         # below rounds up the size of a file/directory, in bytes, to
the
    119         # next multiple of 1024.  This mimics the behavior of ufs
especially
    120         # with directories.  This results in a total size that's
slightly
    121         # bigger than if du was called on a ufs directory.
    122         size32=$(cat "$list32" | xargs -I {} ls -lLd "{}" | nawk '
    123                 {t += ($5 % 1024) ? (int($5 / 1024) + 1) * 1024 : $5}
    124                 END {print int(t * 1.10 / 1024)}')
    125         (( size32 += dirsize32 ))
    126         size64=$(cat "$list64" | xargs -I {} ls -lLd "{}" | nawk '
    127                 {t += ($5 % 1024) ? (int($5 / 1024) + 1) * 1024 : $5}
    128                 END {print int(t * 1.10 / 1024)}')
    129         (( size64 += dirsize64 ))
    130         (( total_size = size32 + size64 ))

Erm... AFAIK the use of xargs/nawk/etc. could be replaced by ksh93
builtin math...

    131 }
    132 
    133 #
    134 # Copies all desired files to a target directory.  One argument
should be
    135 # passed: the file containing the list of files to copy.  This
function also
    136 # depends on several variables that must be set before calling:
    137 #
    138 # $ALT_ROOT - the target directory
    139 # $compress - whether or not the files in the archives should be
compressed
    140 # $rdmnt - the target directory
    141 #
    142 function copy_files
    143 {
    144         list="$1"

Please make "list" a local vairable, e.g. use
-- snip --
typeset list="$1"
-- snip --

    145 
    146         #
    147         # If compress is set, the files are gzip'd and put in the
correct
    148         # location in the loop.  Nothing is printed, so the pipe and
cpio
    149         # at the end is a nop.
    150         #
    151         # If compress is not set, the file names are printed, which
causes
    152         # the cpio at the end to do the copy.
    153         #
    154         while read path
    155         do
    156                 if [ $compress = yes ]; then
    157                         dir="${path%/*}"
    158                         mkdir -p "$rdmnt/$dir"
    159                         /usr/bin/gzip -c "$path" > "$rdmnt/$path"
    160                 else
    161                         print "$path"
    162                 fi
    163         done <"$list" | cpio -pdum "$rdmnt" 2>/dev/null
    164 }
    165 
    166 #
    167 # The first argument can be:
    168 #
    169 # "both" - create an archive with both 32-bit and 64-bit
binaries
    170 # "32-bit" - create an archive with only 32-bit binaries
    171 # "64-bit" - create an archive with only 64-bit binaries
    172 #
    173 function create_ufs
    174 {
    175         which=$1
    176         archive=$2
    177         lofidev=$3

Plese use quotes and make these local variables...

    178 
    179         # should we exclude amd64 binaries?
    180         if [ "$which" = "32-bit" ]; then
    181                 rdfile="$rdfile32"
    182                 rdmnt="$rdmnt32"
    183                 list="$list32"
    184         elif [ "$which" = "64-bit" ]; then
    185                 rdfile="$rdfile64"
    186                 rdmnt="$rdmnt64"
    187                 list="$list64"
    188         else
    189                 rdfile="$rdfile32"
    190                 rdmnt="$rdmnt32"
    191                 list="$list32"
    192         fi

Erm... wouldn't be a case/esac switch better in this case ?

[snip]
    366 #
    367 # This loop creates the 32-bit and 64-bit lists of files.  The
32-bit list
    368 # is written to stdout, which is redirected at the end of the
loop.  The
    369 # 64-bit list is appended with each write.
    370 #
    371 cd "/$ALT_ROOT"
    372 find $filelist -print 2>/dev/null | while read path
    373 do
    374         if [ $SPLIT = no ]; then
    375                 print "$path"
    376         elif [ -d "$path" ]; then
    377                 if [ $format = ufs ]; then
    378                         size=`ls -lLd "$path" | nawk '
    379                             {print ($5 % 1024) ? (int($5 / 1024) + 1) * 
1024 : $5}'`
    380                         if [ `basename "$path"` != "amd64" ]; then
    381                                 (( dirsize32 += size ))
    382                         fi

AFAIK another case where ksh93 builtin math could be helpfull...

    383                         (( dirsize64 += size ))
    384                 fi
    385         else
    386                 filetype=`LC_MESSAGES=C file "$path" 2>/dev/null |\
    387                     awk '/ELF/ { print $3 }'`
    388                 if [ "$filetype" = "64-bit" ]; then
    389                         print "$path" >> "$list64"
    390                 elif [ "$filetype" = "32-bit" ]; then
    391                         print "$path"
    392                 else
    393                         # put in both lists
    394                         print "$path"
    395                         print "$path" >> "$list64"
    396                 fi
    397         fi
    398 done >"$list32"
    399 
    400 if [ $format = ufs ] ; then
    401         # calculate image size
    402         getsize
    403 
    404         # check to see if there is sufficient space in tmpfs 
    405         #
    406         tmp_free=`df -b /tmp | tail -1 | awk '{ printf ($2) }'`
    407         (( tmp_free = tmp_free / 2 ))
    408 
    409         if [ $total_size -gt $tmp_free  ] ; then

Please replace this with...
-- snip --
if (( total_size > tmp_free )) ; then
---- snip ----
... and so on... in general the script's performance can be improved via
enabling the ksh93 builtin commands for "mv", "rm", "basename" and
"dirname".

I can make a "blind" patch (e.g. I can make it but I can't fully test it
on my side) if there is any interest...

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) roland.mainz at nrubsig.org
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 7950090
 (;O/ \/ \O;)

Reply via email to