Remove duplicated code from AtomicString::fromUTF8()
Created attachment 81106 [details] Patch
Attachment 81106 [details] did not build on win: Build output: http://queues.webkit.org/results/7693522
Created attachment 81119 [details] Patch
Attachment 81106 [details] did not build on win: Build output: http://queues.webkit.org/results/7689800
@darin: ping
Comment on attachment 81119 [details] Patch r- b/c this causes a efl build break. Ideally that would be fixed as long as it is possible.
Created attachment 86263 [details] Patch
Comment on attachment 86263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86263&action=review > Source/JavaScriptCore/wtf/text/AtomicString.h:182 > +inline AtomicString AtomicString::fromUTF8(const char* characters, size_t length) > +{ > + if (!characters) > + return nullAtom; > + if (!length) > + return emptyAtom; > + return fromUTF8Internal(characters, characters + length); > +} > + > +inline AtomicString AtomicString::fromUTF8(const char* characters) > +{ > + if (!characters) > + return nullAtom; > + if (!*characters) > + return emptyAtom; > + return fromUTF8Internal(characters, 0); > +} Inlining more like this is a big deal. It will probably make code size a bit bigger because of more checks at every single callsite. When called with a constant C literal, I think the code might get constant-folded away, which would be great. I’m not sure about making this change without checking if there is performance impact.
(In reply to comment #8) > When called with a constant C literal, I think the code might get constant-folded away, which would be great. I'm not sure if we use UTF-8 strings in the code. ;-) > I’m not sure about making this change without checking if there is performance impact. If I remeber correctly it was a slight performance increase, but I'll test it again and post the values here before landing.
Comment on attachment 86263 [details] Patch (In reply to comment #9) > > I’m not sure about making this change without checking if there is performance impact. > If I remeber correctly it was a slight performance increase, but I'll test it again and post the values here before landing. I'v run the xml-parser performance test and this change seams like a 1% performance improvement, but the variation of the results is too big to verify if that's true. :-)
Comment on attachment 86263 [details] Patch Clearing flags on attachment: 86263 Committed r83407: <http://trac.webkit.org/changeset/83407>
All reviewed patches have been landed. Closing bug.