66877 - prefs files need to be consolidated

Status: RESOLVED DUPLICATE of bug 507288

(Core :: Preferences: Backend, defect)

Description

[Judson Valeski]

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.

Comment 1

[Chris McAfee]

adding perf keyword, added to performance page.

Comment 2

[Blake Ross]

I think we should do this for beta1, since we rushed ahead with 62755.

Comment 3

[Judson Valeski]

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).

Comment 4

[timeless]

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.

Comment 5

[Daniel Veditz [:dveditz]]

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.

Comment 6

[(not reading, please use [e-mail] instead)]

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?

Comment 7

[Alec Flett]

I like seth's idea - how about something to indicate that they are "old" prefs,
like "oldprefs.js"?

Comment 8

[Judson Valeski]

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.

Comment 9

[Marek Z. Jeziorek]

* proposing Mozilla0.9 as the target milestone
* adding embed keyword

Comment 10

[Alec Flett]

Hey bug, long time no see.. cc'ing dp cuz he was just in my cube talkin bout this.

Comment 11

[Suresh Duddi (gone)]

We read about 10 pref files on startup costing about 5% of startup. BAD!

[10] 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

Comment 12

[Alec Flett]

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.

Comment 13

[Daniel Veditz [:dveditz]]

Note this is even worse on commercial which has aim.js and a bunch of *-ns.js files

Comment 14

[Winnie Lam]

minusing to topembed- and keeping embed as per edt triage.  not related to a
particular embedding customer.

Comment 15

[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 16

[Daniel Veditz [:dveditz]]

*** Bug 168515 has been marked as a duplicate of this bug. ***

Comment 17

[Juan José Mata]

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?

Comment 18

[Michael Buckland]

Discussed in edt.  Reassigning.

Comment 19

[Judson Valeski]

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.