It turns out that literally 99% of our calls to CopyChars2To2 are to copy only one char. By avoiding a memcpy call for that simple one char case we can avoid roughly half the memcpy calls we make during startup and new window creation. I'm seing about 100,000 memcpy calls at startup skipped with this patch. They were cheap calls since only one char was being copied, but avoiding calls is good. FWIW, The counterpart CopyChars1To1 call very rarely copies only one char. The places in string code using memmove don't seem to get called enough to matter. It looks like this functionality is going to migrate to nsCharTraits::copy (right?). I think it will be worth adding this shortcut there when the code changes over.
Created attachment 57396 [details] [diff] [review] CopyChars2To2 patch to avoid memcpy call most of the time
Hmmm. Who is doing all these 1 char calls?
Looks like nsCSSScanner calling nsStr::StrAppend from three different methods.
Ah, looks like nsCSSToken::AppendToString is the main function doing this. I wonder if it would be better to optimize |nsAString::do_AppendFromElement( char_type aChar )| and its override, |nsString::do_AppendFromElement( PRUnichar inChar )|, instead. Better in the sense that we're not adding an |if| to CopyChars2to2, and better in that we're addressing this issue for both old string code and new string code.
Any news on that one?!
The code in my patch has been running in my tree for a long time. Jag: unless you are going to pursue a better patch soon, why not get mine into the tree in the mean time rather than blowing off the whole thing for the promise of something better?
I'm all for checking your patch in. sr=jag on the patch. Wanna take the bug?
taking. Anyone for an r= ?
Assignee: scc → jband
Comment on attachment 57396 [details] [diff] [review] CopyChars2To2 patch to avoid memcpy call most of the time Recording jag's sr= (or r= as module owner), and my r/sr=. /be
I checked in my patch. Reassign to jag in case he wants to do a more general fix as he suggested.
Assignee: jband → jaggernaut
Target Milestone: --- → Future
Is there any further work on that bug still intended?
Comment on attachment 57396 [details] [diff] [review] CopyChars2To2 patch to avoid memcpy call most of the time This code is obsolete now!
Attachment #57396 - Attachment is obsolete: true
nsCSSToken::AppendToString(nsString& aBuffer) now uses the new string stuff, so this patch is obsolete. Marking 'WONTFIX'
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.