Bug 96132 - Avoid strlen() when converting DTOA result to String
Summary: Avoid strlen() when converting DTOA result to String
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
Depends on: 96130
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-07 10:33 PDT by Patrick R. Gansterer
Modified: 2013-01-18 10:47 PST (History)
9 users (show)

See Also:


Attachments
Patch (12.24 KB, patch)
2012-09-07 10:46 PDT, Patrick R. Gansterer
darin: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.