Darren J Moffat wrote:
> 
> I don't see how the fix under review for this bug can be improving
> performance by using ksh93.  Which is the rationale in the bug for
> switching nightly to ksh93.

In my original review request email I wrote this:
-- snip --
- This patch is _not_ intended to improve performace. This patch only
switches "nightly.sh" from ksh88 to ksh93 to be in sync with "bldenv.sh"
that users can actually use ksh93 statements in the "opensolaris.sh"
config file and to prepare for later cleanup steps (my fear is that this
patch will trigger a larger discussion again and a large patch may
suffer from bitrot again (at least my last two nightly cleanup patches
suffered that fate and I am not exactly willing anymore to write patches
for /dev/null)). As said I now try to split the original monster-patch
into smaller pieces to get them added to the tree faster.
-- snip --

This patch is only to do the switch and do some tiny low-risk things
which directly jump into my eyes and do the real work _later_ (and get
the discussion "why should we use ksh93 ?" now - whichis basically the
main part of the patch). I already tried large-scale all-in-one patches
twice which both suffered the fate from irreparable bitrot and I am not
really willing anymore to write patches for /dev/null. That's why I try
to split the changes into smaller, easier pieces to speed-up code
review.

> There are a few saved processes by chaning 'sort | uniq' constructs to
> 'sort -u' but that can be done independent of ksh93.
> 
> Is there really that big a saving from using the builtins for:
> 
>    63 builtin basename
>    64 builtin cp
>    65 builtin dirname
>    66 builtin mkdir
>    67 builtin mv
>    68 builtin rm
>    69 builtin rmdir
> 
> Or does chaning foo() {} to function foo { }  actually make ksh93
> perform significantly better some how and keep the code meaning exactly
> the same ?

See
http://www.opensolaris.org/os/project/shell/shellstyle/#use_ksh_style_function_syntax

> Why not also use the builtins for: cat, head ?  They aren't used much in
> nightly but they are used.

"cat" is always enabled by default in ksh93...

> Where is the performance improvement coming from that is unique to
> ksh93?  and how much of an improvement is there ?

As said this patch does not aim at performance - I'll do that later (as
a side-note: The script is I/O-dominated in many places and therefore
fixing the places which trigger the { |open()|, |stat()|, |lseek()|,
write()|, |close()| }-storms to |write()| ([1]) will greatly improve the
performace on NFS filesystems. Getting rid of the |fork()|-bomb
behaviour in other places will help a lot, too (e.g. the massive "egrep"
chain somewhere in the script can be reduced to one array and one (or
more (assuming this helps Niagara machines)) egrep statement ([2]) and
other places scream for improvements, too. Loudly.)).

[1]=See
http://www.opensolaris.org/os/project/shell/shellstyle/#group_identical_redirections_together
,
http://www.opensolaris.org/os/project/shell/shellstyle/#use_dynamic_file_descriptors
and
http://www.opensolaris.org/os/project/shell/shellstyle/#use_inline_here_documents
for the fix

[2]=Short example (for ksh93 [3]): The following can be used to bundle
multiple "egrep" statements in one single statement:
-- snip --
typeset -a bundle

bundle+= ( 'foo|bar' )
bundle+= ( '.*mix.*' )
bundle+= ( '^pattern1' )
bundle+= ( 'pattern2$' )

# print array element values, concaternated with IFS[0]
(IFS="|" ; print "(${bundle[*]})")
-- snip --
... this script outputs:
-- snip --
(foo|bar|.*mix.*|^pattern1|pattern2$)
-- snip --
which can directly be fed to "egrep". ~~60 filter processes saved (the
remaining filter chain will allow sufficient parallism, don't worry...).

[3]=BTW: This part is from 2007 when I wrote the first (and now
bitrotted) "nightly.sh" cleanup patch...

> Do any of these changes mean that nightly no longer works with ksh88 ?

Yes, that's the _main_ point, e.g. to have this discussion _now_ and to
avoid that I write a large patch and have it "rejected" then or ending
in a multi-month discussion loop which causes bitrot again (see above)
...

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) roland.ma...@nrubsig.org
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 3992797
 (;O/ \/ \O;)
_______________________________________________
opensolaris-code mailing list
opensolaris-code@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/opensolaris-code

Reply via email to