Bug 96330 - Add String::numberToStringFixedWidth()
Summary: Add String::numberToStringFixedWidth()
Status: RESOLVED FIXED
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: 96264
Blocks:
  Show dependency tree / graph
 
Reported: 2012-09-10 15:15 PDT by Patrick R. Gansterer
Modified: 2012-09-20 14:03 PDT (History)
2 users (show)

See Also:

Attachments
Patch (6.40 KB, patch)
2012-09-10 15:26 PDT, Patrick R. Gansterer 		no flags Details | Formatted Diff | Diff
Patch (7.89 KB, patch)
2012-09-14 14:15 PDT, Patrick R. Gansterer 		no flags Details | Formatted Diff | Diff
Patch (5.81 KB, patch)
2012-09-18 13:54 PDT, Patrick R. Gansterer 		no flags Details | Formatted Diff | Diff
Patch (7.00 KB, patch)
2012-09-18 14:04 PDT, Patrick R. Gansterer 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Patch (7.08 KB, patch)
2012-09-19 04:35 PDT, Patrick R. Gansterer 		no flags Details | Formatted Diff | Diff
Patch (7.92 KB, patch)
2012-09-19 13:40 PDT, Patrick R. Gansterer 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2012-09-10 15:15:39 PDT 
Add String::numberToFixedPrecisionString()
Comment 1 Patrick R. Gansterer 2012-09-10 15:26:36 PDT 
Created attachment 163230 [details]
Patch
Comment 2 Patrick R. Gansterer 2012-09-14 14:15:21 PDT 
Created attachment 164225 [details]
Patch
Comment 3 Benjamin Poulain 2012-09-14 14:26:17 PDT 
Please benchmark JSC's NumberPrototype.

It is important code. From a similar change I did in String, I expect this to have visible perf improvement.
Comment 4 Patrick R. Gansterer 2012-09-14 14:30:02 PDT 
(In reply to comment #3)
> Please benchmark JSC's NumberPrototype.
> 
> It is important code. From a similar change I did in String, I expect this to have visible perf improvement.

IMHO it won't have any visible perf improvement until i remove the strlen() calls in bug 96132, but I'll check...
Comment 5 Benjamin Poulain 2012-09-14 14:46:14 PDT 
> IMHO it won't have any visible perf improvement until i remove the strlen() calls in bug 96132, but I'll check...

You make two important changes for performance:
1) remove one branch
2) remove one parameter

It does not look like much, but that code is in JSC's runtime. This kind of code is frequently exercised in benchmarks.
Comment 6 Build Bot 2012-09-14 16:47:10 PDT 
Comment on attachment 164225 [details]
Patch

Attachment 164225 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13855598
Comment 7 Patrick R. Gansterer 2012-09-18 13:54:44 PDT 
Created attachment 164616 [details]
Patch
Comment 8 Patrick R. Gansterer 2012-09-18 14:00:00 PDT 
(In reply to comment #5)
> > IMHO it won't have any visible perf improvement until i remove the strlen() calls in bug 96132, but I'll check...
> 
> You make two important changes for performance:
> 1) remove one branch
> 2) remove one parameter
> 
> It does not look like much, but that code is in JSC's runtime. This kind of code is frequently exercised in benchmarks.

Changing the NumberPrototype.cpp to use the String::number() showed a (small) performance regression in a micro micro benchmark. I don't think this will be measurable in usual benchmark suites, but I removed the change from this patch. I will improve the String::number() function in a next step (e.g. bug 96132) first, before changing NumberPrototype. But I still see value in this patch, since it introduces the final API already.
Comment 9 Patrick R. Gansterer 2012-09-18 14:04:05 PDT 
Created attachment 164618 [details]
Patch
Comment 10 Build Bot 2012-09-18 14:42:32 PDT 
Comment on attachment 164618 [details]
Patch

Attachment 164618 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13892323
Comment 11 Patrick R. Gansterer 2012-09-19 04:35:13 PDT 
Created attachment 164710 [details]
Patch
Comment 12 Patrick R. Gansterer 2012-09-19 13:40:54 PDT 
Created attachment 164772 [details]
Patch
Comment 13 WebKit Review Bot 2012-09-20 14:03:34 PDT 
Comment on attachment 164772 [details]
Patch

Clearing flags on attachment: 164772

Committed r129165: <http://trac.webkit.org/changeset/129165>
Comment 14 WebKit Review Bot 2012-09-20 14:03:39 PDT 
All reviewed patches have been landed.  Closing bug.