263959 - [FIX]Reuse CSS scanners, not just CSS parsers

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

This comes from the profile in bug 229391.  In that profile, we're spending a
good bit of time (comparable to total time spent in painting, basically) in
InitScanner() and ReleaseScanner(), because we have thousands of inline style
rules being set in a row.

The patch coming up makes CSS parsers not actually delete the scanner when they
ReleaseScanner() and makes scanners deal with being reinitialized.  It saves us
about 1% of the total time on the testcase in the URL field.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 2

[Boris Zbarsky [:bzbarsky]]

Note to self: the line in nsCSSScanner.h that declares mURI is now:

  nsCOMPtr<nsIURI> mURI;  // Cached so we know to not refetch mFileName

per grammar nit on irc.

Comment 3

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

Comment on attachment 161824 [details] [diff] [review]
Patch

I'm not convinced ClearScanningState deserves to be its own function.  It
doesn't really clear all the state, so it doesn't seem useful to callers other
than Init, and there is only one caller.

Also, is there any reason CSSParserImpl::mScanner needs to be a pointer?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch updated to review comments

Comment 5

[Boris Zbarsky [:bzbarsky]]

Fixed

Comment 6

[jag (Peter Annema)]

So, maybe I'm missing something, but it looks like you're never setting
mScannerInited to true.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Good catch.  Checked in the fix.