Bug 96330

Summary:

Add String::numberToStringFixedWidth()

Product:

WebKit

Reporter:

Patrick R. Gansterer <paroga>

Component:

Web Template Framework

Assignee:

Patrick R. Gansterer <paroga>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

96264

Bug Blocks:

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	buildbot: commit-queue-
Patch	none
Patch	none

Patrick R. Gansterer

Reported 2012-09-10 15:15:39 PDT

Add String::numberToFixedPrecisionString()

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 attachment proposed patch, testcase, etc.

Patrick R. Gansterer

Comment 1 2012-09-10 15:26:36 PDT

Created attachment 163230 [details] Patch

Patrick R. Gansterer

Comment 2 2012-09-14 14:15:21 PDT

Created attachment 164225 [details] Patch

Benjamin Poulain

Comment 3 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.

Patrick R. Gansterer

Comment 4 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...

Benjamin Poulain

Comment 5 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.

Build Bot

Comment 6 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

Patrick R. Gansterer

Comment 7 2012-09-18 13:54:44 PDT

Created attachment 164616 [details] Patch

Patrick R. Gansterer

Comment 8 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.

Patrick R. Gansterer

Comment 9 2012-09-18 14:04:05 PDT

Created attachment 164618 [details] Patch

Build Bot

Comment 10 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

Patrick R. Gansterer

Comment 11 2012-09-19 04:35:13 PDT

Created attachment 164710 [details] Patch

Patrick R. Gansterer

Comment 12 2012-09-19 13:40:54 PDT

Created attachment 164772 [details] Patch