stringToLengthType() in SVGLength.cpp is currently testing String::endsWith("foo") which causes temporary WTF::Strings to be created. We can easily hand-compare the characters to speed up this function.
Created attachment 68042 [details] Proposed patch This patch yields a 10.5% speedup on http://data.xeoh.net/svg.benchmark/
(In reply to comment #1) > Created an attachment (id=68042) [details] > Proposed patch > > This patch yields a 10.5% speedup on http://data.xeoh.net/svg.benchmark/ L_Great_TM :)
Comment on attachment 68042 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=68042&action=prettypatch > WebCore/svg/SVGLength.cpp:97 > + if (characters[length - 1] == '%') Can you put UChar b = characters[length - 1]; right before this check? The names should indicate the content. Can you use 'lastChar' for b or simply 'end'? a also needs a better name. Just a bit strange that we give back number, if the string doesn't end with a length type that we know. > WebCore/svg/SVGLength.cpp:104 > + UChar a = characters[length - 2]; > + UChar b = characters[length - 1]; Should be const. I was looking at setValueAsString, the caller of this function. And it looks bogus to me, to just check the last or last two chars. Following example would parse on WebKit, while it doesn't on other Viewers: <rect width="20 mm" height="20*SomethinStupid*px"/> Can't you just use 'ptr' and 'end' of setValueAsString? 'ptr' is the first char after the parsed number of the string. We don't need String at all on lengthTypeToString. This change will need new layouttests.
Created attachment 68733 [details] Proposed patch v2 Strictified SVGLength to only accept unit types immediately following the numeric value, and with no whitespace after. This behavior matches Firefox, but not IE9 or Opera. IE9 accepts whitespace on both sides of the unit type. Opera doesn't accept whitespace between the numeric value and the unit type, but accepts it after the unit type.
Comment on attachment 68733 [details] Proposed patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=68733&action=review The code is not realy faster with the mentioned changes, but looks more like the rest of the parsing code in SVGParserUttilities.cpp. And it may make it easier to change the code to ignore whitespaces at the end without parsing error later (the default behavior on Opera, but not specified yet) by just adding two more lines. The test looks great! Great patch! > WebCore/svg/SVGLength.cpp:89 > +inline SVGLengthType stringToLengthType(const UChar* characters, const UChar* end) const UChar*& ptr > WebCore/svg/SVGLength.cpp:94 > + unsigned length = end - characters; > + > + if (!length) > + return LengthTypeNumber; if (ptr == end) return LengthTypeNumber; > WebCore/svg/SVGLength.cpp:105 > + if (length != 1 && length != 2) > + return LengthTypeUnknown; > + > + const UChar lastChar = characters[length - 1]; > + > + if (length == 1) { > + if (lastChar == '%') > + return LengthTypePercentage; > + return LengthTypeUnknown; > + } const UChar firstChar = *ptr; ++ptr; if (firstChar == '%') { if (ptr == end) return LengthTypePercentage; return LengthTypeUnkown; } const UChar secondChar = *ptr; if (++ptr != end) return LengthTypeUnkown;
Created attachment 68761 [details] Proposed patch v3 Updated with Dirk's suggestions.
Comment on attachment 68761 [details] Proposed patch v3 LGTM. r=me.
Comment on attachment 68761 [details] Proposed patch v3 Clearing flags on attachment: 68761 Committed r68307: <http://trac.webkit.org/changeset/68307>
All reviewed patches have been landed. Closing bug.