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