From a9de50bf79318a2522fd77142df073df2080336a Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 15 Jun 2020 20:07:05 +0200 Subject: [PATCH] Apply simple optimisation for ${ subshare; } (re: 3d38270b) Running shbench after undoing the incorrect subshell optimisation showed that the performance of ${ subshare; }-type command substitutions went down very slightly, but consistently. The main purpose of using this ksh-specific type of command substitution vs. a normal one is performance. Thus, it *is* appropriate to eke every last bit of performance out of it that we can, provided correctness is completely preserved. It is also a type of command substitution where every change is supposed to be shared with the main shell environment; only command output is captured in a subshell-like fashion. Thus, on the face of it, it would be a logical optimisation for sh_assignok() to avoid bothering with saving a subshell context for variables if we're in a subshare. Lo and behold, applying it does not introduce any regress fails. Here are my average results of the braces.ksh benchmark from shbench against stock /bin/ksh 93u+ vs. current 93u+m (same compiler flags), 100 runs pre-optimisation and 100 runs post-optimisation: Stock /bin/ksh: Pre-optimisation (at 3d38270b): 93u+: 0.743 secs 93u+m: 0.739 secs Stock /bin/ksh: Post-optimisation (now): 93u+: 0.744 secs 93u+m: 0.726 secs The left column shows only a small margin of error with 100 runs; the right one shows a very small, but not insignificant difference. However, these tests were not very rigorous with 100 runs each. If anyone wants to do it properly, please report results to korn-shell@googlegroups.com. I'm happy enough with this, though. Thanks to Joerg van den Hoff for providing shbench, without which it would not have occurred to me to try this. src/cmd/ksh93/sh/subshell.c: sh_assignok(): - Don't bother if we're in a ${ subshare; }. --- src/cmd/ksh93/sh/subshell.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index ef33fb3b7..9a920db09 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -248,6 +248,9 @@ Namval_t *sh_assignok(register Namval_t *np,int add) Namval_t *mpnext; Namarr_t *ap; int save; + /* don't bother to save if in a ${ subshare; } */ + if(sp->subshare) + return(np); /* don't bother with this */ if(!sp->shpwd || np==SH_LEVELNOD || np==L_ARGNOD || np==SH_SUBSCRNOD || np==SH_NAMENOD) return(np);