[zypp-devel] Re: [zypp-commit] r9774 - /trunk/zypper/src/zypper-utils.cc
Hi Josef, On Tue, 22 Apr 2008, jreidinger@svn.opensuse.org wrote:
Author: jreidinger Date: Tue Apr 22 12:07:08 2008 New Revision: 9774
URL: http://svn.opensuse.org/viewcvs/zypp?rev=9774&view=rev Log: move replace_all to zypp and change inplace replace to gsub, which have better performance
Your own testcase said something else: zypper gsub boost user time: 769 system time: 0 jano replace_all boost user time: 689 system time: 0 boost replace_all boost user time: 8626 system time: 6 gsub is slower. That's also expected, as it needs to copy the whole string. Ciao, Michael. -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Michael Matz wrote:
Hi Josef,
On Tue, 22 Apr 2008, jreidinger@svn.opensuse.org wrote:
Author: jreidinger Date: Tue Apr 22 12:07:08 2008 New Revision: 9774
URL: http://svn.opensuse.org/viewcvs/zypp?rev=9774&view=rev Log: move replace_all to zypp and change inplace replace to gsub, which have better performance
Your own testcase said something else:
zypper gsub boost user time: 769 system time: 0 jano replace_all boost user time: 689 system time: 0 boost replace_all boost user time: 8626 system time: 6
gsub is slower. That's also expected, as it needs to copy the whole string.
Ciao, Michael.
I post it in another note about performance of different string size to replace, which I before forget send. In zypper is replaced '\n' for some ' ' (not exactly set how many). testcase when to and from have different size: zypper gsub boost user time: 817 system time: 1 jano replace_all boost user time: 830 system time: 0 -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Hi, On Tue, 22 Apr 2008, Josef Reidinger wrote:
gsub is slower. That's also expected, as it needs to copy the whole string.
Ciao, Michael.
I post it in another note about performance of different string size to replace, which I before forget send. In zypper is replaced '\n' for some ' ' (not exactly set how many).
testcase when to and from have different size: zypper gsub boost user time: 817 system time: 1 jano replace_all boost user time: 830 system time: 0
I think that's just an artifact. gsub and replace_all use mostly the same inner loop, on the same inputs: one .find call, one .replace call. gsub just also hoisted the constant length arguments out of the loop body. So gsub does strictly more work than replace_all (the string copy). Ciao, Michael. -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Michael Matz napsal(a):
Hi,
On Tue, 22 Apr 2008, Josef Reidinger wrote:
gsub is slower. That's also expected, as it needs to copy the whole string.
Ciao, Michael. I post it in another note about performance of different string size to replace, which I before forget send. In zypper is replaced '\n' for some ' ' (not exactly set how many).
testcase when to and from have different size: zypper gsub boost user time: 817 system time: 1 jano replace_all boost user time: 830 system time: 0
I think that's just an artifact. gsub and replace_all use mostly the same inner loop, on the same inputs: one .find call, one .replace call. gsub just also hoisted the constant length arguments out of the loop body. So gsub does strictly more work than replace_all (the string copy).
Ciao, Michael.
Interesting, I think gsub works little different so I try implement little different version. Problem with replace on string with different length of substitute is that each call must move some char in internall char strorage...so if gsub instead of replacing append can be better...something like 'non-matching string'+s/from/to/+'another non-matching'...appending can have better performance because doesn't need moving with internal characters like if you change 2char for one, then every characters after this occurence must be moved to left by one. I test it on Thursday or Friday and write here result. Pepa -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Hi, On Wed, 23 Apr 2008, josef reidiner wrote:
Interesting, I think gsub works little different so I try implement little different version. Problem with replace on string with different length of substitute is that each call must move some char in internall char strorage...so if gsub instead of replacing append can be better
This is true, but not how gsub currently is implemented. If you do that, then yes, gsub might potentially be faster.
I test it on Thursday or Friday and write here result.
Cool. Ciao, Michael. -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Michael Matz wrote:
Hi,
On Wed, 23 Apr 2008, josef reidiner wrote:
Interesting, I think gsub works little different so I try implement little different version. Problem with replace on string with different length of substitute is that each call must move some char in internall char strorage...so if gsub instead of replacing append can be better
This is true, but not how gsub currently is implemented. If you do that, then yes, gsub might potentially be faster.
I test it on Thursday or Friday and write here result.
Cool.
Ciao, Michael.
So I implement it (code of it in test file which I attach). Speed improve is really significant. here is my test results: (different size change : for ;; ) new gsub user time: 377 system time: 2 jano replace_all user time: 810 system time: 1 (same size change : for ;) (this quite surprise me) new gsub user time: 364 system time: 1 jano replace_all user time: 530 system time: 3 (remove - change : for ;) new gsub user time: 368 system time: 0 jano replace_all user time: 622 system time: 2 So what you recommend??? remove replace_all and have only new gsub or stay both? welcome any comments on code Pepa
Hi, On Thu, 24 Apr 2008, Josef Reidinger wrote:
So I implement it (code of it in test file which I attach). Speed improve is really significant.
Well, nice, but it doesn't do what it's supposed to. You probably want to check some results manually. In particular:
while (string::npos != (loc = sData.find(sFrom, loc))) { sNew.append(sData,oldLoc,loc);
The third argument to append is a length, not an offset.
sNew.append(sTo); loc += toLen; oldLoc = loc; if (loc >= sNew.length()) break;
Get rid of this if and break. It's meanwhile wrong I think. Ciao, Michael. -- To unsubscribe, e-mail: zypp-devel+unsubscribe@opensuse.org For additional commands, e-mail: zypp-devel+help@opensuse.org
Michael Matz wrote:
Hi,
On Thu, 24 Apr 2008, Josef Reidinger wrote:
So I implement it (code of it in test file which I attach). Speed improve is really significant.
Well, nice, but it doesn't do what it's supposed to. You probably want to check some results manually. In particular:
while (string::npos != (loc = sData.find(sFrom, loc))) { sNew.append(sData,oldLoc,loc);
The third argument to append is a length, not an offset.
sNew.append(sTo); loc += toLen; oldLoc = loc; if (loc >= sNew.length()) break;
Get rid of this if and break. It's meanwhile wrong I think.
Ciao, Michael.
Yes, you are right and also one more bug in code (I add toLen, but need frLen). So now after fix results is not so exciting. Conclusion is, that new gsub is better then old and replace_all is better then new_gsub except for longer 'to' string. (but if you need new string and not replacing in-place then new gsub is better). Fixed code of tests and new gsub is attached. same size of replacing strings: new gsub user time: 570 system time: 0 jano replace_all user time: 554 system time: 1 old gsub replace_all user time: 629 system time: 1 to is bigger: new gsub user time: 794 system time: 2 jano replace_all user time: 861 system time: 2 old gsub replace_all user time: 838 system time: 1 from is bigger: new gsub user time: 404 system time: 0 jano replace_all user time: 353 system time: 2 old gsub replace_all user time: 417 system time: 2
participants (3)
-
josef reidiner -
Josef Reidinger
-
Michael Matz