Replace WTF::numberTo*String() with String::number[ToStringECMAScript]()
Created attachment 162805 [details] Patch
Comment on attachment 162805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162805&action=review > Source/JavaScriptCore/ChangeLog:3 > + Replace WTF::numberTo*String() with String::number[ToStringECMAScript]() Why?
Comment on attachment 162805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=162805&action=review On NumberPrototype and V8DependentRetained, I don't think we can afford one extra function call + the extra branch in String::number(). > Source/WebCore/css/CSSPrimitiveValue.cpp:1028 > - NumberToStringBuffer buffer; > - const char* alphaString = numberToFixedPrecisionString(color.alpha() / 255.0f, 6, buffer, true); > - result.append(alphaString, strlen(alphaString)); > + String alphaString = String::number(color.alpha() / 255.0f); > + result.append(alphaString.characters8(), alphaString.length()); This is also a regression. Previously, we would only allocate NumberToStringBuffer on the stack. With this change, we will allocate a new StringImpl, then copy the content of the buffer to it. I am also not a fan of relying on the implementation generating a 8bits buffer. It is the case today but that could change in the future. Given that someone took the time to write the original code is such a contrived way, I assume the performance is important here. > Source/WebCore/html/parser/HTMLParserIdioms.cpp:90 > - NumberToStringBuffer buffer; > - return String(numberToString(number, buffer)); > + return String::numberToStringECMAScript(number); This looks like a good idea. It makes the code simpler and will enjoy https://bugs.webkit.org/show_bug.cgi?id=96132 for free. > Source/WebCore/platform/Decimal.cpp:685 > - if (isfinite(doubleValue)) { > - NumberToStringBuffer buffer; > - return fromString(numberToString(doubleValue, buffer)); > - } > + if (isfinite(doubleValue)) > + return fromString(String::numberToStringECMAScript(doubleValue)); Ditto.
Created attachment 163014 [details] Patch
Comment on attachment 163014 [details] Patch Clearing flags on attachment: 163014 Committed r128008: <http://trac.webkit.org/changeset/128008>
All reviewed patches have been landed. Closing bug.