597291 - Lazily resolve URLs and other CSS values

Status: VERIFIED FIXED

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

Description

[(dormant account)]

This should be an easy win due to how slow our url code is.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So we can either add a boolean flag to nsCSSValue::URL to indicate whether it's been resolved or add a new unit to nsCSSValue.  The latter doesn't add a word of bloat to URL, but probably requires more callsites to be updated.  David, preference?

We also need to add a base URI to nsCSSValue::URL.  We can probably reuse the existing URI field for this, so this won't increase the size.

Comment 2

[(dormant account)]

(In reply to comment #1)
> So we can either add a boolean flag to nsCSSValue::URL to indicate whether it's
> been resolved or add a new unit to nsCSSValue.  The latter doesn't add a word
> of bloat to URL, but probably requires more callsites to be updated.  David,
> preference?

dbaron, waiting on your input on above question

Comment 3

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

I'd say add a boolean to nsCSSValue::URL.  We can probably also use it to simplify the image-loading stuff.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: zip file with two testcases testing lots of url rules (one about:, one chrome:)

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Create nsIURI objects lazily for nsCSSValue::URL, so that we don't pay the cost of creating the ones we don't actually need. the new setup, the mURL member of nsCSSValue::URL stores either the actual URI pointed to or the base URI; a boolean flag keeps

Comment 6

[Boris Zbarsky [:bzbarsky]]

On the attached testcases, an opt Mac build on my hardware without the patch shows times of about 470ms for bar.html and 370ms for baz.html.

With the patch, those become 150ms and 170ms respectively.

Comment 7

[(dormant account)]

(In reply to comment #6)
> On the attached testcases, an opt Mac build on my hardware without the patch
> shows times of about 470ms for bar.html and 370ms for baz.html.
> 
> With the patch, those become 150ms and 170ms respectively.

win! Is there a css benchmark we can add a the lots of urls testcase to?

Comment 8

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

What was the end of comment 5 supposed to say?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment 5 was auto-generated by bzexport; it's just the checkin comment in the patch.

Taras, we don't have a useful css benchmark.  :(

Comment 10

[Wladimir Palant]

Tested Boris' try build - normally the nsIStyleSheetService.loadAndRegisterSheet() call in Adblock Plus takes 260-300ms for me (almost 1 MB of CSS), with this patch it is down to 70ms.

Comment 11

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

Comment on attachment 526117 [details] [diff] [review]
Create nsIURI objects lazily for nsCSSValue::URL, so that we don't pay the cost of creating the ones we don't actually need.   the new setup, the mURL member of nsCSSValue::URL stores either the actual URI pointed to or the base URI; a boolean flag keeps

r=dbaron

Comment 12

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/d5728312d756

Comment 13

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue using the attached test cases added in Comment 4. Tested on Win XP, Win 7, Mac OS X 10.6 and Ubuntu 10.10

The time shown on FF 6.0b2 is:

- for Win XP: 138ms for bar.html and 179ms for baz.html (On Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110511 Firefox/6.0a1, time was: 583ms for bar.html and 404ms on baz.html)

- for Win 7: 221ms for bar.html and 287ms for baz.html (On Mozilla/5.0 (Windows NT 6.1; rv:6.0a1) Gecko/20110510 Firefox/6.0a1, time was 1069ms for bar.html and 779ms for baz.html.

- for Mac OS X 10.6: 224ms for bar.html and 259ms for baz.html (on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110510 Firefox/6.0a1, time was 850ms on bar.html and 622ms for baz.html

- for Ubuntu 10.10: 235ms for bar.html and 279ms for baz.html (on Mozilla/5.0 (X11; Linux i686; rv:6.0a1) Gecko/20110510 Firefox/6.0a1, time was 1038ms for bar.html and 772ms for baz.html.

Setting status of this issue to VERIFIED FIXED.