103912 - nsChromeRegistry::LoadInstallDataSource() is 2.5% of main1()

Status: VERIFIED FIXED

(Core Graveyard :: RDF, defect, P2)

Description

[Suresh Duddi (gone)]

Timing analysis on this:

main1
				1	0	3,279ms	mozilla.exe
nsChromeRegistry::CheckForNewChrome
1
0ms
63ms
chrome.dll
It doesn't look like CheckForNewChrome() is doing anything. It is taking about
2% of main1() on startp

Can we remove it ?

Comment 1

[David Hyatt]

No, it can't be removed.  This is the function that does chrome registration.

Comment 2

[Suresh Duddi (gone)]

Ah! So if I look at its descendents, almost all of its time is consumed by:

LoadInstallDataSource()
100%
1
63ms

And that inturn is 15 LoadDataSource() calls. Can we merge those individual
files statically instead of loading them and merging them on the fly ?

If you dont see any optimizations here, feel free to INVALID / WONTFIX the bug.

Comment 3

[David Hyatt]

I don't think merging them would buy us much of anything.  A better optimization
IMHO would be to look into why loading RDF/XML files takes so long.

Comment 4

[Suresh Duddi (gone)]

Changing summary approriately

Comment 5

[Suresh Duddi (gone)]

Taking over bug. We agreed to look at the parser first to see if we can improve
its performance. We will revisit this next.

Comment 6

[Chase Tingley]

In my debug build, the time spent in calls to LoadDataSource() is dominated by
loading the first five datasources from dist/bin/chrome.  Strangely enough, the
time spent on these doesn't seem to correspond to the size/complexity of the RDF
file -- user-skins.rdf consistently takes twice as long as all-skins.rdf,
despite being a good deal simpler.

Hmm.

Comment 7

[Suresh Duddi (gone)]

Out of the 47 calls to rdf blocking parser, these are the 11 files that exist:

 455   /export/dp/trunk/mozilla/dist/bin/chrome/user-skins.rdf
5669   /export/dp/trunk/mozilla/dist/bin/chrome/all-skins.rdf
1223   /export/dp/trunk/mozilla/dist/bin/chrome/user-locales.rdf
8194   /export/dp/trunk/mozilla/dist/bin/chrome/all-locales.rdf
8870   /export/dp/trunk/mozilla/dist/bin/chrome/all-packages.rdf
 525  
/export/dp/trunk/mozilla/dist/bin/chrome/overlayinfo/navigator/content/overlays.rdf
1108  
/export/dp/trunk/mozilla/dist/bin/chrome/overlayinfo/messenger/content/overlays.rdf
 291  
/export/dp/trunk/mozilla/dist/bin/chrome/overlayinfo/editor/content/overlays.rdf
2158  
/export/dp/trunk/mozilla/dist/bin/chrome/overlayinfo/communicator/content/overlays.rdf
5937   /home/dp/.mozilla/Default%20User/bxr32616.slt/localstore.rdf

Comment 8

[Suresh Duddi (gone)]

Attachment: urls for 47 calls to rdf blocking parse

Comment 9

[Suresh Duddi (gone)]

Chase, the timing for user skins includes load of liburiloader.so hence the
bloate d time.

Comment 10

[David Hyatt]

This can probably get a bit smaller if we peel out some of the lesser-accessed
RDF into a separate RDF file that is only loaded when showing the prefs UI.

Comment 11

[Suresh Duddi (gone)]

Hyatt already did a lot of improvement by merging the rdf files into one. Need
to measure one last time to make sure and close this out.

Comment 12

[Suresh Duddi (gone)]

I see footprint wins here. The data sources that arent needed actually create a
a hashtable and 3 arenas each. I almost have a patch that will fix this.

Comment 13

[Suresh Duddi (gone)]

I had it stat and prevent the creation of datasources. Hyatt has a better way do
to this: at chrome registration time note down if there exists stylesheets.rdf
and overlays.rdf

Hyatt if you already have a bug, close this as duped and ensure you have that
one nominated (my vote).

Comment 14

[selmer (gone)]

Please update this bug with an [adt1] - [adt3] impact rating (or take it off the
list if it doesn't even rate adt3.)  Thanks!

Comment 15

[David Hyatt]

Attachment: Patch to annotate and avoid creating DS when they don't exist.

Comment 16

[Brian Ryner (not reading)]

Comment on attachment 77539 [details] [diff] [review]
Patch to annotate and avoid creating DS when they don't exist.

r=bryner

Comment 17

[Blake Ross]

Comment on attachment 77539 [details] [diff] [review]
Patch to annotate and avoid creating DS when they don't exist.

sr=blake

Comment 18

[Blake Ross]

Comment on attachment 77539 [details] [diff] [review]
Patch to annotate and avoid creating DS when they don't exist.

sr=blake

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 77539 [details] [diff] [review]
Patch to annotate and avoid creating DS when they don't exist.

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 20

[David Hyatt]

Fixed.

Comment 21

[Tom Everingham]

verified