Summary: | CSS color parsing optimizations | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, eric, oliver | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Andreas Kling
2010-07-12 05:52:43 PDT
Created attachment 61214 [details]
Proposed patch
Comment on attachment 61214 [details]
Proposed patch
Do you have perf measurements for this? There was a canvas perf test from hixie that the #color optimisation was made for, are we sure it doesn't work? or was it regressed at some point?
(In reply to comment #2) > was it regressed at some point? This regressed with https://bugs.webkit.org/show_bug.cgi?id=38845 where I unintentionally removed your optimization from https://bugs.webkit.org/show_bug.cgi?id=3781 Hixies test: http://hixie.ch/tests/adhoc/perf/video/002.html Without this patch (current trunk): Elapsed wall-clock time: 1325ms (ideal: 640ms). Elapsed non-idle time: 685ms (ideal: 0ms). Speed: 11.32fps (ideal: 25.00fps). With this patch: Elapsed wall-clock time: 1011ms (ideal: 640ms). Elapsed non-idle time: 371ms (ideal: 0ms). Speed: 14.84fps (ideal: 25.00fps). Comment on attachment 61214 [details]
Proposed patch
I would prefer it if you had a separate patch to revert the #color parsing regression
(In reply to comment #5) > I would prefer it if you had a separate patch to revert the #color parsing regression I'm not sure we want to revert that. Condensing the color parsing to one call (CSSParser::parseColor()) from the Canvas side is still desirable IMO. Better to fix parseColor() to take the appropriate fast-path instead of doing the Color(string) trick in a bunch of places. Created attachment 61243 [details]
Proposed patch v2
Updated patch to also avoid allocating a new string when getting the RRGGBB (or RGB) part of a '#' color.
Elapsed wall-clock time: 996ms (ideal: 640ms).
Elapsed non-idle time: 356ms (ideal: 0ms).
Speed: 15.06fps (ideal: 25.00fps).
Comment on attachment 61243 [details] Proposed patch v2 Clearing flags on attachment: 61243 Committed r63099: <http://trac.webkit.org/changeset/63099> All reviewed patches have been landed. Closing bug. Comment on attachment 61243 [details]
Proposed patch v2
Adding an include of PlatformString.h to Color.h is not correct. Why was that done?
(In reply to comment #10) > Adding an include of PlatformString.h to Color.h is not correct. Why was that done? For UChar. What is the correct way? wtf/unicode/Unicode.h is how you get UChar. Right, thanks. Filed bug 42109 with a fix. |