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

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: 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).

Comment 2

[Boris Zbarsky [:bzbarsky]]

And of course putting 'px', 'em', 'ex', and 'pt' first would cover almost all of the use cases...

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Using a loop

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 232657 [details] [diff] [review]
Using a loop

r+sr=dbaron

Comment 5

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 6

[Tuukka Tolvanen (sp3000)]

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

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed that.