Opening/closing the various prefs files at startup is expensive. It causes an open/read/close on each file. Some script should run at the end of the build (some post build process) to cobble all of the prefs that were sent to 'dist', into a single (or fewer) file.
adding perf keyword, added to performance page.
I think we should do this for beta1, since we rushed ahead with 62755.
Keywords: perf → nsbeta1
Severity: normal → major
Keywords: mozilla0.8, perf
OS: Windows 2000 → All
Hardware: PC → All
On the surface the fix for this seems to be a perl script that takes *.js files in the pref file dist dir, and simply appends them to all.js. The perl script is obviously cake... tieing this into a post build step is the tricky part (from where I sit).
i'm not commmitting a fix for bug 62755 until this is implemented. personally, i'd prefer for us to handle modularity well, so if someone installs say mailnews.xpi then the install.js could ask that mailnews.js be added to all.js similarly for whatever modules we have. In normal dists, the build system would be responsible for the merging, but the unmerged files would still be available.
Summary: prefs files need to be consolodated → prefs files need to be consolidated
I originally put the mailnews.js default into the mail install package figuring you didn't need them if you didn't install mailnews, and promptly broke profile migration in doing so. The mail folks insisted that all default prefs ship with the browser. Tying this into the *build* step, so that whatever pref files are generated by the build get combined, could be a good thing. Probably not something that belongs to me, though. I'm doing Quantify runs now anyway, I'll check this out and see if the time is spent opening files (which this would help) or parsing the pref contents (which this wouldn't) before we bother to do anything.
thanks for remembering, dveditz. if you installed navigator only, migration code would fail because it relied on mailnews.js values to do the right thing. (even though you couldn't run mail, we'd still migrate over your mail files, in case you went and got mail later.) how about having a migration.js or something. I could put all the mailnews prefs (needed for migration) in there, so we won't have to worry about mailnews.js. comments?
I like seth's idea - how about something to indicate that they are "old" prefs, like "oldprefs.js"?
adding comments from dveditz: "You'd want to believe simply catting the pref files would work, but it won't. We will also have to change the pref code. Prefs specifically loads "initprefs.js" first and will fail if it's not there. Then it loads whatever it finds, then explicitly loads <platform>.js last and will fail if that's not found. In the middle you also can't just willy-nilly concatenate prefs because our *-ns.js pref files absolutely depend on being processed AFTER their related mozilla pref files so they can override defaults. A lot of this is cruft which could go away, but would take someone's time to sort through." So, we special case the initprefs.js, cat all the "stuff in the middle", and special case the platform.js file. The NS tree can cat all it's "stuff in the middle" too, and in the NS tree build step, we can just ensure that the NS "stuff in the middle" gets _appended_ to the mozilla "stuff in the middle" that yields 3 total files, which is obviously better off than where we are today. special case files can be left out, point is the "stuff in the middle" should get consolodated in the end.
* proposing Mozilla0.9 as the target milestone * adding embed keyword
Keywords: embed, mozilla0.9
Hey bug, long time no see.. cc'ing dp cuz he was just in my cube talkin bout this.
We read about 10 pref files on startup costing about 5% of startup. BAD!  List of pref files loaded: ---------------------------------------------------------------------- 1. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\initpref.js 2. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\xpinstall.js 3. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\security-prefs.js 4. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\mailnews.js 5. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\editor.js 6. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\config.js 7. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\all.js 8. e:\dp\opt.trunk\mozilla\dist\WIN32_O.OBJ\bin\defaults\pref\winpref.js 9. C:\Documents and Settings\dp\Application Data\Mozilla\Profiles\Default User\l1vrcf3h.slt\prefs.js 10. C:\Documents and Settings\dp\Application Data\Mozilla\Profiles\Default User\l1vrcf3h.slt\user.js
initpref.js is an autoconfig thing, and will require bug 89137 to be fixed first. Once that is fixed, there is no reason to use JS to read in a user's prefs. dp, bnesse, and I talked about this, and we'd like to eventually switch to some other format no longer based on JS, like the nsIPersistentProperties file. We should probably open a few bugs for that.
Depends on: 89137
Note this is even worse on commercial which has aim.js and a bunch of *-ns.js files
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.8
minusing to topembed- and keeping embed as per edt triage. not related to a particular embedding customer.
Keywords: topembed → topembed-
Please update this bug with an [adt1] - [adt3] impact rating (or take it off the list if it doesn't even rate adt3.) Thanks!
*** Bug 168515 has been marked as a duplicate of this bug. ***
EDT triage giving this a topembed+. This is a potential performance win but is in direct conflict with bug 62755 which we just gave a topembed+ as well. Can we make it so that the consolidation happens at packaging time? Will that still give us the desired outcome from the aforementioned bug 62755?
Keywords: topembed- → topembed+
Discussed in edt. Reassigning.
Assignee: dveditz → nobody
Status: ASSIGNED → NEW
Target Milestone: mozilla1.2alpha → ---
after talking this over w/ alec, minusing. the relative perf/footprint win in the embedding context isn't high enough to justify the lifting needed here.
Keywords: topembed+ → topembed-
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 507288
You need to log in before you can comment on or make changes to this bug.