[FIX]Would it be faster to not call LookupKeyword in TranslateDimension?

RESOLVED FIXED in mozilla1.9alpha1

Status

()

P3
normal
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

({perf})

Trunk
mozilla1.9alpha1
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

12 years ago
Calls to LookupKeyword via TranslateDimension are about 1% of total time on the testcase at <http://www.speich.net/computer/3d.php>.  For comparison, all of reflow is about 4-5%.

I wonder whether we could avoid calling LookupKeyword here altogether...
(Assignee)

Comment 1

12 years ago
Created attachment 232499 [details] [diff] [review]
Partial impl using switch cascade

This is really hard to read and doesn't seem like a good approach, but it _is_ faster.

Frankly, I'm almost tempted by a linear search down the unit list (with the most common units up front).  There are few enough units that this might be faster than the nsCSSKeywords lookup (esp. because the latter ends up doing string copies).
(Assignee)

Comment 2

12 years ago
And of course putting 'px', 'em', 'ex', and 'pt' first would cover almost all of the use cases...
(Assignee)

Updated

12 years ago
Keywords: perf
(Assignee)

Comment 3

12 years ago
Created attachment 232657 [details] [diff] [review]
Using a loop
Assignee: dbaron → bzbarsky
Attachment #232499 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #232657 - Flags: superreview?(dbaron)
Attachment #232657 - Flags: review?(dbaron)
(Assignee)

Updated

12 years ago
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Updated

12 years ago
Summary: Would it be faster to not call LookupKeyword in TranslateDimension? → [FIX]Would it be faster to not call LookupKeyword in TranslateDimension?
Comment on attachment 232657 [details] [diff] [review]
Using a loop

r+sr=dbaron
Attachment #232657 - Flags: superreview?(dbaron)
Attachment #232657 - Flags: superreview+
Attachment #232657 - Flags: review?(dbaron)
Attachment #232657 - Flags: review+
(Assignee)

Comment 5

12 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
fwiw, added build noise:

In member function ‘PRBool CSSParserImpl::TranslateDimension(nsresult&, nsCSSValue&, PRInt32, float, const nsString&)’:
http://lxr.mozilla.org/mozilla/source/layout/style/nsCSSParser.cpp#3611
warning: comparison between signed and unsigned integer expressions

3610     PRInt32 i;
3611     for (i = 0; i < NS_ARRAY_LENGTH(UnitData); ++i) {

s/PRInt32/PRUint32/ helps
(Assignee)

Comment 7

12 years ago
Fixed that.
You need to log in before you can comment on or make changes to this bug.