Bug 96130 – Replace WTF::numberToString() with String::numberToStringECMAScript()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Patrick R. Gansterer]

Replace WTF::numberTo*String() with String::number[ToStringECMAScript]()

[Patrick R. Gansterer]

Created attachment 162805 [details]
Patch

[Darin Adler]

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?

[Benjamin Poulain]

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.

[Patrick R. Gansterer]

Created attachment 163014 [details]
Patch

[WebKit Review Bot]

Comment on attachment 163014 [details]
Patch

Clearing flags on attachment: 163014

Committed r128008: <http://trac.webkit.org/changeset/128008>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.