399349 - nsCSSScanner: Optimize parseNum to handle integer numbers directly

Status: RESOLVED DUPLICATE of bug 498559

(Core :: CSS Parsing and Computation, defect)

Description

[Alfred Kayser]

Attachment: V1: Parse upto 9 digits directly as integer

This patch optimizes number parsing for numbers without a '.' and up to 9 digits directly as an intValue, skipping the 'ident' gathering for such numbers.
All other numbers (more than 9 digits or a '.' present) are handled in the usual way: collect into the ident string, and then do ToFloat.

This way we prevent all kinds of overflow issues, but optimize the most common type of numbers in CSS sheets (e.g. 1px, 2px, 0, etc).

Comment 1

[Alfred Kayser]

Attachment: V2: Only use AppendInt when we have more than 9 digits before the dot

1. normal integers are always directly parsed, no conversion.
2. normal floats (<9 digits before .) only ToFloat for fraction part,
3. huge floats (>9 digits before .) AppendInt and ToFloat for conversion.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I won't get to this until May at best.

Comment 3

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

Comment on attachment 284444 [details] [diff] [review]
V2: Only use AppendInt when we have more than 9 digits before the dot

No tests or information about what was tested ==> review-

Comment 4

[Alfred Kayser]

Attachment: V3: With a bunch of testcases for number parsing

I have created a special testfile with 24 testcases for number parsing.
Together with the testcases in test_parse_rule.html this should cover number parsing of a wide range of scenarios.

Note floating point numbers are stored as 'float' and are thus very limited in precision. Also the CSS code itself massages numbers.

Anyway these testcases caught a number of issues with I have solved with this patch.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Nits:
s/zero's/zeroes
s/numers/numbers

Comment 6

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

Did you test that actually improved performance?  On what testcases?

Comment 8

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

Comment on attachment 305359 [details] [diff] [review]
V3: With a bunch of testcases for number parsing

Clearing review request on obsolete patch; looks like Boris already landed the test.