Bug 96132

Summary:

Avoid strlen() when converting DTOA result to String

Product:

WebKit

Reporter:

Patrick R. Gansterer <paroga>

Component:

Web Template Framework

Assignee:

Patrick R. Gansterer <paroga>

Status:

NEW ---

Severity:

Normal

CC:

benjamin, dglazkov, ggaren, gustavo, msaboff, peter+ews, philn, webkit.review.bot, xan.lopez

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

96130

Bug Blocks:

Attachments:

Description	Flags
Patch	darin: review-, buildbot: commit-queue-

Description Patrick R. Gansterer 2012-09-07 10:33:33 PDT

Avoid strlen() when converting DTOA result to String

Comment 1 Patrick R. Gansterer 2012-09-07 10:46:54 PDT

Created attachment 162814 [details]
Patch

Comment 2 Benjamin Poulain 2012-09-07 11:00:49 PDT

This is very interesting

Since it touches JSC's runtime. Could you please check SunSpider to ensure there are no regressions? Better safe than getting yelled at ;)

Comment 3 Build Bot 2012-09-07 11:20:29 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13777684

Comment 4 Build Bot 2012-09-07 11:30:05 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13776782

Comment 5 Early Warning System Bot 2012-09-07 12:07:02 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13774835

Comment 6 Early Warning System Bot 2012-09-07 12:08:43 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13772829

Comment 7 Gyuyoung Kim 2012-09-07 12:22:52 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13786551

Comment 8 Peter Beverloo (cr-android ews) 2012-09-07 12:42:56 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13777711

Comment 9 WebKit Review Bot 2012-09-07 15:37:16 PDT

Comment on attachment 162814 [details]
Patch

Attachment 162814 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13788308

Comment 10 Darin Adler 2013-01-18 10:47:05 PST

Comment on attachment 162814 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162814&action=review

review- because patch doesn’t build and so needs some additional work; probably should review a version that does build

> Source/JavaScriptCore/runtime/NumberPrototype.cpp:388
> +    WTF::NumberToStringBuffer buffer;

The WTF:: prefix should not be needed here.

> Source/WTF/wtf/dtoa.h:47
> -WTF_EXPORT_PRIVATE const char* numberToString(double, NumberToStringBuffer);
> -WTF_EXPORT_PRIVATE const char* numberToFixedPrecisionString(double, unsigned significantFigures, NumberToStringBuffer, bool truncateTrailingZeros = false);
> -WTF_EXPORT_PRIVATE const char* numberToFixedWidthString(double, unsigned decimalPlaces, NumberToStringBuffer);
> +WTF_EXPORT_PRIVATE int numberToString(double, NumberToStringBuffer);
> +WTF_EXPORT_PRIVATE int numberToFixedPrecisionString(double, unsigned significantFigures, NumberToStringBuffer, bool truncateTrailingZeros = false);
> +WTF_EXPORT_PRIVATE int numberToFixedWidthString(double, unsigned decimalPlaces, NumberToStringBuffer);

This return value is a lot less self-explanatory than the old one. We probably need a comment or some other form of explanation. Perhaps this should even be an out argument instead of a return value for clarity.