Closed
Bug 311485
Opened 19 years ago
Closed 19 years ago
StyleVariantToColor could use better string handling
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
(Keywords: perf)
Attachments
(1 file)
1.45 KB,
patch
|
vlad
:
review+
darin.moz
:
superreview+
|
Details | Diff | Splinter Review |
This came up in a profile for bug 297959. The breakdown for time spent under
nsCanvasRenderingContext2D::StyleVariantToColor is:
14334 CSSParserImpl::ParseColorString
3383 XPCVariant::GetAsWStringWithSize
2110 nsSubstring::Assign
978 nsAString_internal::~nsAString_internal()
406 NS_Free_P
358 nsVariant::ConvertToWStringWithSize
and small fry. Note all the string-munging. What we're doing (when you look at
the GetAsWStringWithSize method on variants) is:
1) Creating a dependent string inside the variant.
2) Doing ToNewUnicode on it.
3) Creating an nsString using the result in the caller
I suggest that instead of calling GetAsWStringWithSize we should just call
GetAsAString here and pass an nsAutoString. That will involve one single string
copy (instead of the two we have here) and probably zero heap allocations
(instead of the two we have here) because color strings are short.
Assignee | ||
Comment 1•19 years ago
|
||
Attachment #198853 -
Flags: superreview?(shaver)
Attachment #198853 -
Flags: review?(vladimir)
Comment on attachment 198853 [details] [diff] [review]
Like so
r=vladimir
Attachment #198853 -
Flags: review?(vladimir) → review+
Comment 3•19 years ago
|
||
Comment on attachment 198853 [details] [diff] [review]
Like so
sr=darin
Too bad we don't have a way to get a dependent string out of a variant.
Attachment #198853 -
Flags: superreview?(shaver) → superreview+
Assignee | ||
Updated•19 years ago
|
Assignee: nobody → bzbarsky
Target Milestone: --- → mozilla1.9alpha
Assignee | ||
Comment 4•19 years ago
|
||
Fixed on trunk
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•