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

VERIFIED FIXED in mozilla1.0

Status

()

Core
RDF
P2
normal
VERIFIED FIXED
17 years ago
4 years ago

People

(Reporter: Suresh Duddi (gone), Assigned: David Hyatt)

Tracking

({memory-footprint, perf})

Trunk
mozilla1.0
x86
Windows 2000
memory-footprint, perf
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt3])

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
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 ?
(Reporter)

Updated

17 years ago
Blocks: 7251
Keywords: perf
(Assignee)

Comment 1

17 years ago
No, it can't be removed.  This is the function that does chrome registration.
(Reporter)

Comment 2

17 years ago
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.
(Assignee)

Comment 3

17 years ago
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.
(Reporter)

Comment 4

17 years ago
Changing summary approriately
Summary: nsChromeRegistry::CheckForNewChrome() Needs to be removed → nsChromeRegistry::LoadInstallDataSource() is 2% of main1()
(Reporter)

Comment 5

17 years ago
Taking over bug. We agreed to look at the parser first to see if we can improve
its performance. We will revisit this next.
Assignee: hyatt → dp
Summary: nsChromeRegistry::LoadInstallDataSource() is 2% of main1() → nsChromeRegistry::LoadInstallDataSource() is 2.5% of main1()
Target Milestone: --- → mozilla0.9.7

Comment 6

17 years ago
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.
(Reporter)

Updated

17 years ago
Priority: -- → P2
(Reporter)

Updated

17 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 7

17 years ago
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

(Reporter)

Comment 8

17 years ago
Created attachment 56345 [details] [diff] [review]
urls for 47 calls to rdf blocking parse
(Reporter)

Comment 9

17 years ago
Chase, the timing for user skins includes load of liburiloader.so hence the
bloate d time.
(Reporter)

Updated

17 years ago
Depends on: 109488
(Reporter)

Updated

17 years ago
Depends on: 109590
(Reporter)

Updated

17 years ago
Target Milestone: mozilla0.9.7 → mozilla0.9.8
(Assignee)

Comment 10

17 years ago
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.
(Reporter)

Updated

17 years ago
Target Milestone: mozilla0.9.8 → mozilla0.9.9
(Reporter)

Comment 11

17 years ago
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.
(Reporter)

Updated

17 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0
(Reporter)

Updated

17 years ago
Keywords: nsbeta1
(Reporter)

Comment 12

17 years ago
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.
Keywords: nsbeta1 → footprint, nsbeta1+
(Reporter)

Comment 13

17 years ago
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).
Assignee: dp → hyatt
Status: ASSIGNED → NEW

Comment 14

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

Updated

16 years ago
Whiteboard: [adt3]
(Assignee)

Comment 15

16 years ago
Created attachment 77539 [details] [diff] [review]
Patch to annotate and avoid creating DS when they don't exist.
Attachment #77539 - Flags: review+
Comment on attachment 77539 [details] [diff] [review]
Patch to annotate and avoid creating DS when they don't exist.

r=bryner

Updated

16 years ago
Attachment #77539 - Flags: superreview+

Comment 17

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

sr=blake

Comment 18

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

sr=blake

Comment 19

16 years ago
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
Attachment #77539 - Flags: approval+
(Assignee)

Comment 20

16 years ago
Fixed.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 21

16 years ago
verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.