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: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Bug Depends on: | 96264 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Patrick R. Gansterer
2012-09-10 15:15:39 PDT
Created attachment 163230 [details]
Patch
Created attachment 164225 [details]
Patch
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. (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... > 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 on attachment 164225 [details] Patch Attachment 164225 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13855598 Created attachment 164616 [details]
Patch
(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. Created attachment 164618 [details]
Patch
Comment on attachment 164618 [details] Patch Attachment 164618 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13892323 Created attachment 164710 [details]
Patch
Created attachment 164772 [details]
Patch
Comment on attachment 164772 [details] Patch Clearing flags on attachment: 164772 Committed r129165: <http://trac.webkit.org/changeset/129165> All reviewed patches have been landed. Closing bug. |