* [PATCH] getstrvalue() optimization @ 2016-11-08 13:37 ` Sebastian Gniazdowski 2016-11-09 12:39 ` Peter Stephenson 0 siblings, 1 reply; 4+ messages in thread From: Sebastian Gniazdowski @ 2016-11-08 13:37 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 803 bytes --] Hello There are double strlen() invocations in getstrvalue(). It's not about negative indexes – although they have been optimized too – but about dupstring() and v->end, v->start verification code at the end. Actually a single strlen() call is required. Attached test script runs 1765 ms for optimized Zsh, 1980 ms for no optimizations (minimum obtainable times): strtest() { a="" i=$(( 4000 )) while (( i -- )); do a+="a${a[1,200]}" done } I've used signed size variable but the code did that already. More optimization is possible – dupstring() could do strncpy(), copy only requested number of bytes, e.g. first 200 out of 10000, but the trailing \0 nuances made me wait with this change. -- Sebastian Gniazdowski psprint@fastmail.com [-- Attachment #2: string_opt.diff --] [-- Type: text/plain, Size: 1504 bytes --] diff --git a/Src/params.c b/Src/params.c index 330f22b..5005042 100644 --- a/Src/params.c +++ b/Src/params.c @@ -2060,6 +2060,7 @@ getstrvalue(Value v) { char *s, **ss; char buf[BDIGBUFSIZE]; + int len = -1; if (!v) return hcalloc(1); @@ -2237,22 +2238,26 @@ getstrvalue(Value v) return s; if (v->start < 0) { - v->start += strlen(s); + v->start += (len=(int)strlen(s)); if (v->start < 0) v->start = 0; } if (v->end < 0) { - v->end += strlen(s); + len = (len >= 0) ? len : (int)strlen(s); + v->end += len; if (v->end >= 0) { char *eptr = s + v->end; if (*eptr) v->end += MB_METACHARLEN(eptr); } } - s = (v->start > (int)strlen(s)) ? dupstring("") : dupstring(s + v->start); + + len = (len >= 0) ? len : (int)strlen(s); + s = (v->start > len) ? dupstring("") : dupstring_wlen(s + v->start, len - v->start); + if (v->end <= v->start) s[0] = '\0'; - else if (v->end - v->start <= (int)strlen(s)) + else if (v->end - v->start <= len - v->start) s[v->end - v->start] = '\0'; return s; diff --git a/Src/string.c b/Src/string.c index 04e7446..b46ea60 100644 --- a/Src/string.c +++ b/Src/string.c @@ -43,6 +43,19 @@ dupstring(const char *s) /**/ mod_export char * +dupstring_wlen(const char *s, unsigned len) +{ + char *t; + + if (!s) + return NULL; + t = (char *) zhalloc(len + 1); + strcpy(t, s); + return t; +} + +/**/ +mod_export char * ztrdup(const char *s) { char *t; [-- Attachment #3: testopt2.zsh --] [-- Type: application/octet-stream, Size: 213 bytes --] #!Src/zsh-opt3 #!Src/zsh-no-opt #!Src/zsh-opt2 #!Src/zsh-opt zmodload zsh/zprof strtest() { a="" i=$(( 4000 )) while (( i -- )); do a+="a${a[1,200]}" done } strtest zprof | head -n 25 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] getstrvalue() optimization 2016-11-08 13:37 ` [PATCH] getstrvalue() optimization Sebastian Gniazdowski @ 2016-11-09 12:39 ` Peter Stephenson 2016-11-10 8:14 ` Sebastian Gniazdowski 0 siblings, 1 reply; 4+ messages in thread From: Peter Stephenson @ 2016-11-09 12:39 UTC (permalink / raw) To: zsh-workers While I'm still looking at this... We always need the string length; there's no early return between the first two cases and where we ensure we have it. pws diff --git a/Src/params.c b/Src/params.c index 772345b..ef72cba 100644 --- a/Src/params.c +++ b/Src/params.c @@ -2060,7 +2060,7 @@ getstrvalue(Value v) { char *s, **ss; char buf[BDIGBUFSIZE]; - int len = -1; + int len; if (!v) return hcalloc(1); @@ -2237,15 +2237,13 @@ getstrvalue(Value v) if (v->start == 0 && v->end == -1) return s; + len = strlen(s); if (v->start < 0) { - len = strlen(s); v->start += len; if (v->start < 0) v->start = 0; } if (v->end < 0) { - if (len < 0) - len = strlen(s); v->end += len; if (v->end >= 0) { char *eptr = s + v->end; @@ -2254,8 +2252,6 @@ getstrvalue(Value v) } } - if (len < 0) - len = strlen(s); s = (v->start > len) ? dupstring("") : dupstring_wlen(s + v->start, len - v->start); ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] getstrvalue() optimization 2016-11-09 12:39 ` Peter Stephenson @ 2016-11-10 8:14 ` Sebastian Gniazdowski 2016-11-10 10:44 ` Peter Stephenson 0 siblings, 1 reply; 4+ messages in thread From: Sebastian Gniazdowski @ 2016-11-10 8:14 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 203 bytes --] Hello, I thought to make some things clear, not sure if without this any problems are possible (tried to obtain bad behavior but Zsh worked normally). -- Sebastian Gniazdowski psprint@fastmail.com [-- Attachment #2: string_fix.diff --] [-- Type: text/plain, Size: 663 bytes --] diff --git a/Src/params.c b/Src/params.c index ef72cba..0d17047 100644 --- a/Src/params.c +++ b/Src/params.c @@ -2060,7 +2060,7 @@ getstrvalue(Value v) { char *s, **ss; char buf[BDIGBUFSIZE]; - int len; + int len, zerol=0; if (!v) return hcalloc(1); @@ -2255,9 +2255,12 @@ getstrvalue(Value v) s = (v->start > len) ? dupstring("") : dupstring_wlen(s + v->start, len - v->start); + if (s[0] == '\0') + zerol = 1; + if (v->end <= v->start) s[0] = '\0'; - else if (v->end - v->start <= len - v->start) + else if (!zerol && v->end - v->start <= len - v->start) s[v->end - v->start] = '\0'; return s; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] getstrvalue() optimization 2016-11-10 8:14 ` Sebastian Gniazdowski @ 2016-11-10 10:44 ` Peter Stephenson 0 siblings, 0 replies; 4+ messages in thread From: Peter Stephenson @ 2016-11-10 10:44 UTC (permalink / raw) To: zsh-workers On Thu, 10 Nov 2016 00:14:24 -0800 Sebastian Gniazdowski <psprint@fastmail.com> wrote: > Hello, > I thought to make some things clear, not sure if without this any > problems are possible (tried to obtain bad behavior but Zsh worked > normally). I don't think this needs a special case. If it did, I think that would mean some assumption in the code was already broken. pws ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-11-10 10:44 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20161108133901epcas4p257eb8a5bd308d2393769c4a3a2fc3731@epcas4p2.samsung.com> 2016-11-08 13:37 ` [PATCH] getstrvalue() optimization Sebastian Gniazdowski 2016-11-09 12:39 ` Peter Stephenson 2016-11-10 8:14 ` Sebastian Gniazdowski 2016-11-10 10:44 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).