Add a fast case for ASCII strings in HashAndUTF8CharactersTranslator::equal
Created attachment 75607 [details] Patch
Comment on attachment 75607 [details] Patch Again, on what benchmark? So much code for unclear gains. Please document your measurement techniques in the ChangeLog.
Comment on attachment 75607 [details] Patch Again, I don’t quite see eye to eye with Eric on this. This optimization looks like a good idea and is not a lot more code. But I’d like to understand more what this makes faster. What platform? What test?
I'm also not sure Darin and I disagree here either. :)
Same as in https://bugs.webkit.org/show_bug.cgi?id=50516#c9 (and sorry for the delay): avg median stdev OS X 10.6 original >4000 >4000 ;-) r74063 2185.72 2182 12.126895728091345 r74063 with patch 2127.83 2118.5 75.47424130125455 -2.65% -2,91% Maybe someone else can confirm the speed increase? I'm not sure if this (and the change in bug 50516 - both) cause such a "huge" performance win.
Comment on attachment 75607 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75607&action=review I think it’s OK to land this patch. I’m going to change the review to review+. Eric, please do let me know if you object. I am not trying to override you. > JavaScriptCore/wtf/text/AtomicString.cpp:235 > + return equalUTF16WithUTF8(string->characters(), string->characters() + string->length(), buffer.characters, buffer.characters + buffer.length); We have stringCharacters in a local variable, and should use it instead of string->characters().
I would like Patrick to update the ChangeLog to explain his testing, and ideally add the benchmark to our repository under WebCore/benchmarks. Without clear (repeatable) testing, optimizing like this makes little sense to me. :) Thank you for reviewing Darin.
Comment on attachment 75607 [details] Patch Eh. Looking at this patch again, it looks clearly right. Still would be good to know what perf testing is being done here.
Created attachment 77465 [details] Patch I'll cq+ this, when the xml parser benchmark is landed.
Comment on attachment 77465 [details] Patch Clearing flags on attachment: 77465 Committed r74829: <http://trac.webkit.org/changeset/74829>
All reviewed patches have been landed. Closing bug.