Closed Bug 597291 Opened 14 years ago Closed 13 years ago

Lazily resolve URLs and other CSS values

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla6

People

(Reporter: taras.mozilla, Assigned: bzbarsky)

References

Details

(Whiteboard: [ts])

Attachments

(2 files)

This should be an easy win due to how slow our url code is.
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.
(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
Assignee: nobody → bzbarsky
I'd say add a boolean to nsCSSValue::URL.  We can probably also use it to simplify the image-loading stuff.
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.
(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?
What was the end of comment 5 supposed to say?
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.  :(
Whiteboard: [ts] → [need review][ts]
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.
Blocks: abp
Priority: -- → P1
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
Attachment #526117 - Attachment description: 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 → 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
Attachment #526117 - Flags: review?(dbaron) → review+
Whiteboard: [need review][ts] → [need landing][ts]
Pushed http://hg.mozilla.org/mozilla-central/rev/d5728312d756
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
OS: Linux → All
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [need landing][ts] → [ts]
Target Milestone: --- → mozilla6
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: