Summary says it all. Code-level breakdown soon. /be

Mostly startup perf win, lesser footprint savings (maybe half of 143K for first browser window?), definitely mozilla0.9 timeframe. /be

One approach: - Hook into nsComponentManagerImpl::AutoRegisterComponents, alongside the xptinfo stuff (and into the installer/regxpcom) = walk through the dist/bin/chrome directories and jars, looking for JS files = compile them, if there's no pre-compiled file as new or newer, in the directory hierarchy, storing chrome://messenger/content/commandglue.js's compiled form in dist/bin/chrome/messenger/content/messenger/commandglue.jsc - Hook into the chrome: loader, to get it to load the .jsc version if it's present, when looking for a *.js chrome: URL. This will cause scripts to not benefit from the JAR-aggregation process, though, which might lead to a whole new kind of performance pain. I guess we'll have to see. We could also teach the make-chrome process to compile the JS files before packaging them, but brendan is rightly concerned about having to freeze the bytecode format, and prefers to have the scripts compiled on the user's machine. Outstanding issues: - Is it OK to store the time-stamp data in the registry? If not, where? - What do we do with themes and skins that are installed in ~/.mozilla? - Where should this enumerate-and-compile code live? xpconnect? js/precompile? The chrome: code? - Do we do the same for JS components? (Why not?)

is the work for this on track?

I'm taking this one back. Update soon, it's my main focus for this week (besides helping get JS1.5 SpiderMonkey RC3 out). /be

Several problems: - The srcnote vector's terminating SRC_NULL note was not being XDR'ed. - JSVAL_NULL and JSVAL_VOID were not handled correctly by js_XDRValue. null would crash under it, in js_XDRObject; void would be confused for a very negative int. Rather than keep compatibility with such bugginess, I just fixed these bugs. I don't think anyone cares, since the only use of js_XDRValue in the engine is for atom keys, and null and undefined are not atomized. But if some 3rd party used jsxdrapi.h, they still couldn't depend on the old behavior for null (cuz of the crash); and for void, they would be depending on a bug where void becomes an int on decode. - The JSClass registry in each JSXDRState was written assuming few classes per XDR stream would have their objects serialized/deserialized. It now uses a double hash table if 10 or more classes are registered and JS_FindClassIdByName is called. (Hey, why isn't that function named JS_XDRFind...?) The only cleanup apart from tab elimination and some line-formatting nitpicking is that I renamed js_XDRNewBase to js_XDRInitBase, because it's a "struct init" function, not a "new" (instance-allocating) function. I'd like to move the cx arg to the end of js_XDRInitBase's parameter list, too. Shaver, any idea whether someone is using this function? How about r= from a JS dude and sr= from shaver? /be

I've known about the JSVAL_NULL and _VOID problems for quite some time, but since that's a Can't Happen case for script serialization, I never bothered to fix them. I seriously doubt that anyone is using js_XDR*. API-frob away! (I think you named JS_FindClassIdByName, actually. =) ) Good fixes all around; I offer my sr= through a fog of shame.

r=rogerl. This also fixes a long open bug, #31003.

sr=hyatt

- Get rid of XXXbe before MEM_NEED macro by calling MEM_LEFT from its body's else clause. - Avoid single-use locals such as cx in mem_finalize. - Reorder args to JS_XDRInitBase so as to avoid giving cx priority, which usually indicates a fallible, error-reporting API entry point but does not here. - JS_XDRUint8 and JS_XDRUint16 were needlessly masking as well as casting down to uint8 and uint16, respectively. - js_XDRString was failing to store zeroes when padding to an aligned boundary. - Avoid noisy /* !IS_BIG_ENDIAN */ comments after #else and #endif that are only a few lines away from their controlling #if. /be

Thanks for all the reviews -- there was a bug in the JS_XDRString address arithmetic that cleared the padding bytes (memset(raw + nbytes - padlen, ...) should have been memset((char *)raw + nbytes - padlen, ...), as raw is jschar*). I'll put patches in the bug and then check 'em in. /be

Step 1 patch checked in. Next step: hack XUL brutal sharing code to save compiled scripts as needed, and load them instead of compiling their sources if it can. /be

rest of this is for 0.9, correct?

Right. /be

What about making mozilla jumpstart chrome file(s)? A jumpstart chrome file should contain the most wanted chrome parts only! The first window will display faster and that's why people think it's much faster then before! I'm sure it can be started just under 5 secs on an average computer system.

Not going to make 0.9, and not worth rushing. /be

The general idea here could speed up startup a fair bit, but it needs more time to design. IOW, I don't want to hack just a JS fast-load format, when we should fast-load all our compiled "code" (XUL, XBL, CSS, JS) from a unified .fasl file. /be

What would be the structure of the .fasl file? I was thinking that it would be nice for these fast load solutions to be independent of each other (eg when only the js engine is used in a piece of software), so would a jar holding individual files be appropriate, whether unified with the existing chrome jars or not?

Precompiling the UI on first startup sounds like a nice thing, but it would be a shame and against the spirit of the web if one day people could just drop in some compiled stuff in the chrome-directory (or much worse: put it on the web), so that other folks don't get access to the sources anymore. Imagine the internet consisting only of Microsoft .chm-files...

See FASTLOAD_20010529_BRANCH for work in progress. Anyone who figures out why the changes there (which save a navigator.mfasl file on Unix in one's profile directory after first startup, which file currently contains serialized scripts, principals and URL objects; this file is read by subsequent startups to recover compiled scripts, successfully AFAICT from watching in gdb) seem to cause a XUL syntax error to be reported (navigator.xul line 318 column 5, unclosed token) gets a secret reward from me. Back in five days. /be

Forgot to update this bug the other day: FASTLOAD_20010529_BRANCH works on Linux now, and builds on Window. Debugging fastload file deserialization problems on Windows now. /be

cathleen said her pull from the branch built and ran more than twice on Windows. Over the phone, I heard her step through nsXULPrototypeScript::Deserialize, so we think this works on Windows too. I'm not sure why dbaron's Win2k build did not work. I think we're almost there -- any Mac help in sight? /be

waterson tried cathleen's optimized windows build on his sucky 200MHz machine and measured a 16% speedup. Woo hoo! Now I need some advice on hacking CSS code to serialize and deserialize its expensive-to-compile in-memory data structures. /be

Re: bad PR_Seek behaviour on Mac, with -ve offset. The code in mac_io.c should do the right thing here (the assertion seems bogus). I suspect that if bad things are happening when the assertion is removed, then we have other problems.

FASTLOAD_20010703_BRANCH is the new branch. I'm rebuilding it now, hope I didn't miss a file or merge badly. Both client.mk and client.mak have been modified to pull the various modules from the branch. Anyone who can help me test this new branch, please have at it. With some Mac help from peterv (much appreciated), sfraser, and saari, I hope to land this branch in a few days. /be

is there somewhere a linux or windows binary to download? I would really like to see this branch :-)

My Mac FASTLOAD_20010703_BRANCH build fails with nsIStreamBufferAccess.h not found. I guess I need to add an idl file to a project somewhere?

I fixed the MANIFEST_IDL files in xpcom/io and xpcom/ds to list all .idl files that I know are live (nsIBaseStream.idl in io should be cvs removed, I think). /be

.idl files have to be added both to the MANIFIEST_IDL and the IDL.mcp project. peterv fixed it.

With the hidden window creation commented out (so this works on Mac), I see a ~6% improvement in warm start time. (cold start was a tad slower). This is on a fast machine (500MHz G4).

I have this built on win2k and I'm seeing two glitches. One, when you open some dialogs what comes up is a full-screen empty window that is completely white, sometimes with a titlebar, sometimes with none at all. Two, probably related, some of the 'mfl' files have a length of 32 bytes, which can't be right.

sfraser: cold start can be measured to show a speedup too, if you define cold as "after reboot, but with a FastLoad file" rather than "without a FastLoad file". Waterson's 16% speedup on his 200MHz machine was cold. Hope I'm not confused; the point is that cold start should speed up too, once the FastLoad file exists. jrgm: the hiddenWindow problem sfraser diagnosed is just the tip of the iceberg! The FastLoad file must evolve to multiplex shared scripts and overlays and help different, racing applications store (on "cold-meaning-no-FastLoad-file-exists") and load them coherently. It'll become more like a dbm file. I'll be hacking this up in the 20010703_BRANCH, shooting to land early next week. Any and all QA and perf measurement help is most welcome. /be

My numbers were 'cold start with fastload file' vs. 'cold start without fastload file', so I think I was doing the correct comparisons. It wasn't significantly slower (in the noise of the test, which has a resolution of 1 sec or so).

FASTLOAD_20010703_BRANCH has been updated (just now) with fixes and extensions to handle multiplexing of documents (master and overlay .xul files) within the FastLoad file and read-or-write process. Please, please help me test this -- I need to land the branch very soon. It works on Linux, and I bet on Windows. It should fix the hiddenWindow.xul vs. navigator.xul battle on the Mac. You should remove any old FastLoad file(s) in your profile directories. Look for the new, unified "XUL FastLoad File" (Mac), XUL.mfasl (Unix), and XUL.mfl (Windows) file, and please suggest a better basename than "XUL". NB: if there is an unexpected error reading the file, e.g., due to it lacking a multiplexed document's scripts, it will be moved aside to "Aborted.mfasl" by DEBUG builds, and removed by release builds. /be

Ok, try again after updating xpcom/io and xpcom/ds on the branch -- Simon helped me find a couple of bugs (nsSupportsArray didn't QI to nsICollection; zero net length or eight byte gross length document segments in the FastLoad multiplex were not skipped by nsFastLoadFileReader::Read). /be

I'm still seeing problems (udpated mozilla/xpcom to the latest). The fastload file is only 320K in size, and I'm getting 'demux underflow' and other assertions on starting up with a fastload file.

Including the two most recent updates, I have an opt. build on win2k. However, :-[ I crash on the second attempt to startup (the one where the .mfl file has already been created).

I blew it, but rev 1.1.4.8 (just checked in) of xpcom/io/nsFastLoadFile.cpp should make things work better. jrgm, give it a try -- it doesn't affet the .mfl file, so as long as the one your build generated is >400KB, you should be good to go. sfraser, I don't know why you have a small FastLoad file -- 320KB is clearly way undersized. Let's debug tomorrow. If you can remove it and try again with the reader fix cited above, please let me know results ASAP. /be

Yep, that works. (My .msf was >400KB as I recall, but for other testing, I'd had to blow that profile away, so I started with a new profile). At any rate, I went through navigator, composer, mailnews, addrbook and opened every dialog (xul) that I could find, and didn't notice anything unusual. I'll work on some timing tomorrow.

Sorry Brendan, still no workee. My fastload file is now 504k, but the subsequent fastload startup asserts about missing docShellElements, and throws up a big blnak window.

Whew. Not only does it transpire that XUL brutal sharing is not enough to share strres.js (e.g.) when it is included via script src= in distinct .xul files (duh on that one -- waterson's old XUL script cache makes a comeback, with necessary changes), but we have to multiplex .js files as we do overlays. Otherwise, on Mac at any rate, hiddenWindow.xul and navigator.xul race variously to serialize shared scripts. If the race run while writing the FastLoad file differs in order from the race run while reading, doomage ensued, prior to the latest round of checkins. Branch buddies, please update, recompile (everything: nsIXULPrototypeCache.h changed), remove any leftover FastLoad file(s) from your profile directories, and test again. Thanks, /be

Sorry, not good. I start OK with no .mfl file, but I have a crash on exit in that initial run like so: JS_STATIC_DLL_CALLBACK(JSDHashOperator) gc_root_marker(JSDHashTable *table, JSDHashEntryHdr *hdr, uint32 num, void *arg) { JSGCRootHashEntry *rhe = (JSGCRootHashEntry *)hdr; jsval *rp = (jsval *)rhe->root; jsval v = *rp; //<-- crash /* Ignore null object and scalar values. */ if (!JSVAL_IS_NULL(v) && JSVAL_IS_GCTHING(v)) { JSContext *cx = (JSContext *)arg; gc_root_marker(JSDHashTable * 0x00b895e0, JSDHashEntryHdr * 0x022096cc, unsigned long 0, void * 0x02549b68) line 891 JS_DHashTableEnumerate(JSDHashTable * 0x00000001, int (JSDHashTable *, JSDHashEntryHdr *, unsigned long, void *)* 0x00be74ea gc_root_marker(JSDHashTable *, JSDHashEntryHdr *, unsigned long, void *), void * 0x02549b68) line 457 + 15 bytes js_GC(JSContext * 0x02549b68, unsigned int 0) line 1114 js_ForceGC(JSContext * 0x02549b68) line 943 + 12 bytes js_DestroyContext(JSContext * 0x00000000, int 2) line 228 JS_DestroyContext(JSContext * 0x02549b68) line 882 + 11 bytes XPCJSContextStack::~XPCJSContextStack(XPCJSContextStack * const 0x00000000) line 56 XPCJSContextStack::`scalar deleting destructor'(XPCJSContextStack * const 0x00000000, unsigned int 1) + 8 bytes XPCPerThreadData::CleanupAllThreads() line 516 + 13 bytes NTDLL! 77f89898() Then, when I go to restart, with the .mfl in place, I just get a big, white window. [Note: I did clear the .mfl out of the way, and I built from the top with the most recent changes to the branch].

Try again -- I whiffed it in the reader, still (not absolutizing relative script src= URLs). However, your crash when writing reeks of bad windows build make dependencies. Can you update, clobber, and rebuild for manana? Thanks, /be

Yeah, I'd begun to suspect that something was wrong with dependencies, and I was in the middle of a clobber now. But, I'll pull your other changes, and restart.

Spoke too soon -- one more fix from me tonight, and I'll test write and read all the way, and report back with a better update/rebuild signal. /be

Testing on Linux here from the branch -- startup seems if not fast then at least appreciably faster, but better yet so does 'new window'. Not bad. Keep going. =)

Hmm. Odd. Sometimes the XUL.mfasl file is created on startup, sometimes it isn't. Sometimes it's removed at shutdown, sometimes it isn't.

adam: you're seeing some symptoms of hairy bugs that I've just fixed, combined with the robustication code that aborts the FastLoad process on write and read, if things seem to be going south. I've just fixed some bugs that are too nasty to explain here. Connoiseurs should check out bonsai for FASTLOAD_20010703_BRANCH. Anyway, as Kirk said in pure ham style in STII:TWOK, "the word is given" -- go test my branch! It works for me on Linux (famous last words). /be ("KKKKKKKKKKKKKHAAAAAAAAAAAAAAAAAAAAAAAANNNNNNNNNNNNNNNNNNN!!!!!!!!!!!!!!!!")

Hey Kirk. Are you holding out on a change to nsFastLoadFile.h, like the declaration of mSkipOffset?

(Too much time with green-skinned chicks and moon princesses, what can I say?) Fixed, grab 1.1.4.4 of xpcom/io/nsFastLoadFile.h. /be

Rightyho... I've updated mozilla (and xpcom) again but the XUL.mfasl file is still being randomly removed at shutdown, and regardless of its existance is ALWAYS being re-created from scratch at startup, which I assume is defeating the point. Linux/x86. Here is my size (oo-er), for reference: -rw-r--r-- 1 fox 417745 Jul 16 09:13 ./default/XUL.mfasl

In my build on win32, the XUL.mfl file is _not_ being removed on exit or subsequent restarts. It is created once if it doesn't exist, and is not modified after that.

Adam, you should probably update in content too.

I did some measurements of the change in startup times. The test passes the startup time in msec into mozilla as part of the command-line URL. The end time is when the onload is fired in a trivial HTML file loaded from disk. It's not the greatest test in the world, but works as a macro measure of everything from "double-click" to initial page load. The app can vary quite a bit in the time it takes to do a cold start, and with only 3 samples (because I'm lazy) for the reboot case, I don't think the deltas measured here are statistically significant (i.e., I don't think this affects startup time differently for a cold start on the 500MHz machine as opposed to the 266MHz machine). # samples FASL 7/15 Trunk 7/15 Delta (sec.) % Change win2k/500MHz/128MB cold boot 3 10.92 10.99 0.07 -0.7% subseq.start 9 5.54 6.03 0.48 -8.0% win98/266MHz/128MB cold boot 3 19.41 20.00 0.60 -3.0% subseq.start 9 6.38 6.96 0.58 -8.4% So, that's on the order of a half-second gain in startup. That understates the speedup produced by this serialization, since there is a chunk of time at the beginning (XPCOM init, DLL loading) and at the end (initial page load) that are independent of this change.

Static build and (based on that) code reorganization to minimize working set are two very promising avenues being explored in parallel with the FASTLOAD work. But I'm hopeful that FASTLOAD will pay off more next week, as hyatt helps me stuff style sheet data into the mux. Waterson's stopwatch measurement on his 200MHz machine showed a much greater (~16%) cold startup win. I wonder how much RAM that machine had? /be

I've re-updated in content, and xpcom and nspr also, for good luck, on top of what's picked up by the branch's client.mk. There was nothing to update since my last comment. I'm pretty sure I've got it all. So my observations stand -- XUL.mfasl is randomly removed at shutdown, and then rebuilt at startup regardless of its current [non]existence.

Are there some good old-fashioned debugging printf()s that I can #define-in to help diagnose the problem, if you don't already have suspicions..?

adam: try enabling the NS_BREAK() in nsXULDocument::AbortFastLoads that's currently #ifdef DEBUG_brendan, and let me know the stack. If you're building optimized, stick a printf in the callsites of that routine -- but the stack and possibly more info available in debug builds would help more (I couldn't tell whether you were running a debug or release build). /be

This is home-built, but without debug, so I traded the NS_BREAK() for an abort() and got the following trace: [snip] #3 0x404ce401 in abort () from /lib/libc.so.6 #4 0x40a1589b in nsXULDocument::AbortFastLoads () from /marian/cvs/mozilla/dist/bin/components/libgkcontent.so #5 0x40a09155 in XULContentSinkImpl::OpenScript () from /marian/cvs/mozilla/dist/bin/components/libgkcontent.so #6 0x40a086ee in XULContentSinkImpl::OpenTag () from /marian/cvs/mozilla/dist/bin/components/libgkcontent.so #7 0x40a065fa in XULContentSinkImpl::OpenContainer () from /marian/cvs/mozilla/dist/bin/components/libgkcontent.so #8 0x407e4b43 in CWellFormedDTD::HandleStartToken () from /marian/cvs/mozilla/dist/bin/components/libhtmlpars.so #9 0x407e4820 in CWellFormedDTD::HandleToken () from /marian/cvs/mozilla/dist/bin/components/libhtmlpars.so #10 0x407e4393 in CWellFormedDTD::BuildModel () from /marian/cvs/mozilla/dist/bin/components/libhtmlpars.so #11 0x407dc6bb in nsParser::BuildModel () from /marian/cvs/mozilla/dist/bin/components/libhtmlpars.so #12 0x407dc44d in nsParser::ResumeParse () from /marian/cvs/mozilla/dist/bin/components/libhtmlpars.so #13 0x407de429 in nsParser::OnDataAvailable () ---Type <return> to continue, or q <return> to quit--- from /marian/cvs/mozilla/dist/bin/components/libhtmlpars.so #14 0x408150db in nsDocumentOpenInfo::OnDataAvailable () from /marian/cvs/mozilla/dist/bin/components/liburiloader.so #15 0x4076a572 in nsJARChannel::OnDataAvailable () from /marian/cvs/mozilla/dist/bin/components/libnecko.so #16 0x4072977b in nsOnDataAvailableEvent::HandleEvent () from /marian/cvs/mozilla/dist/bin/components/libnecko.so #17 0x40729031 in nsARequestObserverEvent::HandlePLEvent () from /marian/cvs/mozilla/dist/bin/components/libnecko.so #18 0x4014a3c8 in PL_HandleEvent () from /marian/cvs/mozilla/dist/bin/libxpcom.so #19 0x4014a2af in PL_ProcessPendingEvents () from /marian/cvs/mozilla/dist/bin/libxpcom.so #20 0x4014b530 in nsEventQueueImpl::ProcessPendingEvents () from /marian/cvs/mozilla/dist/bin/libxpcom.so #21 0x4083f6b4 in event_processor_callback () from /marian/cvs/mozilla/dist/bin/components/libwidget_gtk.so [snip] I also enabled the Aborted.mfasl move, and FWIW the Aborted.mfasl file which gets created at startup when the XUL.mfasl managed to survive from the previous shutdown is identical to the XUL.mfasl which mozilla creates in its place. (Wow, that sentence sucked.) I'll start the slow process of rebuilding choice cuts of mozilla with -g for a better trace now, unless you say 'that's enough!'

The trace made me sniff out my component.reg, and sure enough the code was choking at every startup on (presumably) trying to load obsoleted scripts; nuking component.reg has eliminated the constant rebuild of the mfasl file at startup and neither have I seen a recurrence of the random-deletes-of-file-at-shutdown. HOORAY!

I updated xpcom/io and netwerk/base/src today with some optimizations to stream buffering for faster i/o (input, actually). I'd like to land this week, perhaps Wednesday morning? The CSS-FastLoad work can go on in the trunk, or in a new branch. /be

Seems to work okay on macos, I get a XUL FastLoad File of 420K. Didn't notice much speed improvement on my SCSI G4 500, but this is a debug build so all bets are off. It didn't seem worse in anycase.

I should note that I had to remove the layout versions of nsLayoutAtomList.h and nsXULAtomList.h so my build wouldn't find those first and would use the content shared versions instead. I dunno why I'm the only one seeing that behavior.

saari: can you pull xpcom/io and content/xul/document/src again and rebuild and retest? Just for sanity, my changes today are good AFAICT, and they finally solve a problem jrgm and I have talked about: the my drool-walk-crawl-run theory of FastLoad devlopment had the FastLoad file contain only whatever was serialized on the first startup where no FastLoad file existed (e.g., browser only), and if you started a different set of app components, (e.g. mailnews and browser), the mailnews XUL document that first loaded and failed to find its URI mapped in the multiplex would abort the process and remove/move-aside the file. Just now, I checked in compiling but untested code to handle such hetergenous startup cases. Without adding a whole lot more code, the FastLoad read process continues, but data not deserialized from the file is reparsed/compiled and then serialized as an update. jrgm, go nuts! /be

saari: sfraser saw 5-6% startup improvement on his debug build. We really need to test optimized to get a good measurement. /be

Do we know how smfr was measuring this? I'm guessing with the Apple profiling tool...

saari: have to ask sfraser when he returns from vacation -- sorry. all: don't try starting mailnews after the browser fastloads, using a top-of-branch build -- I need to fix a nasty bug in my grand update code. More later tonight. /be

Whew -- see FASTLOAD_20010703_BRANCH bonsai for what I did, cvs comments and diffs should be intelligible (or I need sleep). Anyway, things look pretty good to me. I ran browser to generate a ~400K .mfasl file, then started mail to get a 700K total size from the update, then started the mail account manager and ended up with a 841K file. I didn't test all the dialogs and prefs to see whether their scripts and event handlers all worked; any help on that front is much appreciated. The FastLoad machinery, including incremental, crash-safe updates, seems to be humming. Let me know of any strangeness. Thanks, /be

brendan: I did a Purify build. Only one problem... UMR because fun_xdrObject does not initialize atomstr in the !JSXDR_ENCODE case before using it in the call to JS_XDRStringOrNull(xdr, &atomstr). Looks like a trivial fix.

With brendan's most recent changes in an opt. build on win2k ... I started up mozilla, creating a brand new profile (initial size 470KB). I then proceeded to creating a mailnews account and then opening every dialog in the application (final size 2,739KB). I didn't note any problems (except, maybe, the first time I used directory.xul [e.g., get a file:// directory listing] the window was blank and did not show a file tree. However, on a second and subsequent attempts, things were fine). I then went through a restart and went back to many of these dialogs with no problems. I then blew away the XUL file (well, copied it elsewhere) and went back through a bunch of dialogs, regenerating the fasl content; no problems. I had some problems with opening mailnews and browser concurrently (pref setting in Prefs->Appearance). I could start composer and mailnews concurrently, but when there was a pre-existing .mfl, I couldn't reliably get navigator to come up (but mailnews always came up). When I tried starting concurrently mailnews and navigator with _no_ pre-existing .mfl file, on one try I got navigator, another try mailnews, and a third try I crashed in nsXULDocument::AbortFastLoads with gFastLoadList being null. However, I could start composer+navigator and composer+mailnews reliably both with and without an existing .mfl file. [Sidenote: who actually uses this feature anyways ...] I also tried manually corrupting the .mfl file, by inserting random characters in it, or delete lines in the middle. The good news was that it started OK in browser; the bad news is that it kept the existing .mfl file ... I thought it would have blown it away. Anyways, that's all for now. Other things to look at: interaction with '-turbo', and checking a full commercial build (AIM,PSM,etc.) although that's not at the top of my list right now.

Does anything have to happen to existing chrome for this to work? Also, does it pick up changes/new installs of chrome properly? I'm going to try and pull the branch monday and build commerical with it to see what happens.

jband: thanks -- fixed. jrgm: I haven't yet got the checksumming code implemented, so random corruption in the middle may not be noticed. It might just hit some script metadata used to map line numbers, e.g., and have fairly innocuous effects. kerz: the FastLoad file format includes make-like dependencies by which the file can be invalidated if any constituent files look newer, but I have not hooked up dependency generation yet. I'll work on that tonight. I'll look into the browser+mailnews race problems now. Thanks for all the help, /be

content/xul/document/src/nsXULDocument.{h,cpp} are updated to handle a case that may have led to the crash jrgm saw (null gFastLoadList, but really: a deleted nsXULDocument still on the list). More in a bit. /be

Purple Alert! If you have rev 3.61.42.1 of js/src/jsfun.c, update it again from the branch and remove your FastLoad file. All should be well after that. Sorry about the trouble. /be

I just pref-disabled-by-default XUL FastLoad -- you'll need to set nglayout.debug.disable_xul_fastload to true in your all.js (but don't check in) or in user.js (I have a change callback set up). Please enable, all ye testers? /be

D'oh! I mean set nglayout.debug.disable_xul_fastload to false, to enable XUL FastLoad. Darnit, /be

I could use more reviews. The patch touches caps, netwerk, and mailnews, for instance. Please help me land this by Tuesday night. /be

mailnews changes look OK, assuming nothing bad is going to happen because those methods are stubbed out.

Looks generally OK to me. Is there any risk of an attacker being able to use principal de-serialization to forge a principal?

bienvenu: stub nsISerializable methods are ok for now, as no mailnews nsIURI subtypes are serialized yet. mstoltz: if the bad guy can get to your FastLoad file, you are doomed. Same as if there were no FastLoad file and the attacker modified a chrome .js file. I am not reducing or increasing security. Argh, darin is probably reading through a huge mail backlog. Cc'ing dougt and gagan (who's here at the conference) for netwerk review. /be

A one quick comment before i digest your buffered stream changes. I am not sure about the nsIURI deriving from a nsISerializable. I think that it only the URI implementations that support serializablity should need to implement it. Thoughts?

What is the need for nsIStreamBufferAccess; doesn't ReadSegments work? (Heck, I don't even see any callers in this patch);

dougt: nsISerializable is on the nsIURI inheritance chain to avoid yet another vptr per URI or URL instance. This may not be a significant savings, but in general if an interface is implemented by a populous object to be serialized (e.g., nsIContent), I think we do better to chain to nsISupports through nsISerializable. The more we move into the FastLoad file, the greater the number of objects that need to support nsISerializable. So I saved a vptr per instance at the cost of three stub methods per URI concrete class. The nsIStreamBufferAccess interface will be used shortly. It is not redundant w.r.t. ReadSegments, because it is meant to be used by nsISerializable::Read and nsISerializable::Write impls directly. Consider 1000 content nodes, each with 10 data members to serialize (made-up numbers, but within a binary order). Instead of 10000 calls to Read32 or Write32, which call through Read or Write on the underlying stream, finally reaching a buffer, we do 1000 GetBuffer calls and 1000 PutBuffer calls and 10000 loads or stores (with byte swapping on little endian platforms). nsIStreamBufferAccess also supports buffer disable and enable methods, so you can use direct i/o for small things (like the FastLoad file header) and avoid dumping a just-filled input buffer. Finally, I moved the SWAP32, etc. macros from nsBinaryStream.cpp to nsIStreamBufferAccess.idl's %{C++...%} block, and provided NS_ prefixes and GET and PUT macros for the primitive types. These are meant to be used with a char* returned by nsIStreamBufferAccess::GetBuffer. Sun XDR has a similar "inline buffer access" scheme, which NFS uses to save cycles and avoid calling through multiple layers, exactly as I propose to do with hot FastLoad data structures. /be

1. I am a little concerned about the issues that doug raised too about making nsIURI be derived from nsISerializable. Since a significant set of nsIURI implementors (JARURI, SimpleURI, etc) don't have (as yet) a useful reason to implement nsISerializable. Seems like QI'ng for nsISerializable (and failing in these cases) would keep it more optimized than calling into no-op functions, no? Also be aware that JARURI, SimpleURI, etc. have a more important concern for performance than other URI types. So having a good reason behind making things have better performance is important. 2. wrt URL's read and write calls, should we worry about version issues and possible incompatibilities arising due to changes in stream format? (ie. someone messing up in how they write and read to and from streams) I am fine with the way it is right now as long as we could ensure that. 3. small nitpick about nsStdURL::GetCID-- is myCID var needed?

Brendan: This seems like very cool stuff :-) As far as this business of whether or not nsIURI should inherit from nsISerializable, I have my doubts. I say this primarily because being serializable is not a necko requirement. Also, nsIURI is a "nearly frozen" interface, which is implemented by code outside the mozilla codebase. Making nsISerializable part of the inheritance chain says to me that its methods must be implemented by all nsIURI implementations, otherwise it is an optional interface and should be accessed via QueryInterface. When I look at your patch, I see that the nsISerializable methods are not implemented by all nsIURI implementations, so I really think that nsIURI should not inherit from nsISerializable. It sounds like there are some URI types that you want to serialize and others that will never need to be serialized. If this is indeed the case, then I really think that QI over inheritance is the answer.

gagan: re (1), I do not see a performance penalty to nsIURI inheriting from nsISerializable, and the stubs are in fact failure-generators, which mean that if you ever try to serialize one of these NOT_IMPLEMENTED URI subtypes, you'll get a fatal serialization error that suggests you need to write the actual code to make these methods succeed. Serializability is viral, it's like const. If eventually, as I expect, nearly all URI subtypes need to be serializable, we won't have so many stubs. Re: (2) the FastLoad file is versioned as to metadata format, and the XUL document code versions its data. I'm working on the checksum support now. Re: (3) myCID is needed -- C++ won't let you assign an aggregate initializer to a lvalue, you can do that only in a declaration. Darin: sold, I did not know nsIURI was frozen (or nearly so). If you don't mind the creeping MI bloat (a word per instance for yet another vptr), I will redo the code to take nsISerializable out of nsIURI's inheritance chain. /be

cc'ing valeski b/c there may be other interfaces that are frozen or nearly frozen.

no other interface melting going on in the patch... just nsIURI. I like the idea of URI's being serializable (though I'm not sure how necessary/practical/useful it is right now. are you seeing some strong gains by making them serializable?), but I'd prefer QI access rather than MI. Why MI?

jud: everything in the XUL prototype document, URIs, scripts, style sheets, and even decoded images, is likely to want to be serializable. I used SI (not MI) at first to avoid yet another vtbl ptr per instance, with QI as usual. The only issue was not whether to QI, it was how to inherit nsISerializable. I've redone things so that the nsIURI implementations reached by XUL prototype docs inherit nsISerializable independently from nsIURI. New patch coming up later tonight, based on current trunk. /be

Okay, so I built with that latest patch and a trunk pulled at 2am last night. It's alive.... 1) I went through all the xul dialogs in mailnews, addrbook, navigator composer and chatzilla (i.e., not including aim, psm, (other?)), and periodically shutting down and restarting. Final .mfl size = 3018KB 2) I didn't note any breakage in any dialog, although I wasn't doing a full functional test for every dialog. But signing up for new news or mail accounts, setting various preferences, replying/forwarding mail, adding address card, filing bookmarks, adding sidebars, etc. were all working fine. 3) I had previously (10 days ago) seen an occasional shutdown crash, but never hit it with this build (maybe some fix by dbaron for js gc roots). 4) I could start with any concurrent combination of mailnews|composer|navigator (This had not worked reliably before) So, looking good. To try this out (once this patch hits the trunk) set this in prefs.js: 'user_pref("nglayout.debug.disable_xul_fastload", false);' Still to do: evil file corruption tests, and timing tests.

I've set the pref in prefs.js and launched browser + mail + preferences dialogs but the size of XUL.mfl is only 4K whatever I do I'm running a home build on Win2k from latest sources and it is a static build questions to save some time: Is it supposed to work with a static build ? Do I need to create a new profile (mine is a bit old) ? Should I put the pref in all.js also ?

Bernard: you need to apply the latest patch in this bug ("trunk-based patch ..."), and it sounds like you have not done that. /be

Hmm. Bug 92859 has a patch that, when merged up and checked in, may make my patch to caps/src obsolete. I'll help with that and work on checksumming, and perhaps come back with a better patch still before going for review. If all you testing buddies can limp along by appling the latest patch here (the "slightly nicer patch") until then, I'll be ever so grateful. Sorry this isn't working out of the box, but if vidur's patch in 92859 fixes things so I don't have to make caps create principals using CreateInstance, I think it's worth the wait. If the wait requires too much patch applying by interested parties, I'll check in the interim solution. /be

Sorry for the spam. /be

Revising dependency. Bug 93792 has a patch that relieves caps/src/ code from having to CreateInstance its own concrete types. I'll get that in, then redo a patch for this bug. /be

Adding to CC - sorry for the spam.

This bug has mozilla0.9.2 as a keyword. Update or remove?

Looking for dbaron r= and waterson/jst/hyatt sr=. /be

I crave shaver's review, as always! bugmail spam gets him to give it up, usually. /be

Sorry, I missed a crucial step in nsFastLoadFileWriter::Close (storing the fresh checksum, so it can be retrieved later via nsIFastLoadFileControl::GetChecksum and cached in the service). I also took this opportunity to eliminate an XXXbe comment in nsFastLoadService::StartMuxedDocument, by making the downcast from nsIObjectInputStream* to nsFastLoadFileReader* in NS_NewFastLoadFileUpdater type-safe, using ye-olde-QI-to-CID hack. Bletch! But it works and is minimal. /be

the netwerk changes seem fine for the most part, but can you explain nsISeekableOutputStream? why the change from |fill| to |read|? why can't nsIInputStream be used in place of this interface?

darin, the short answer is a question: do you really want nsFileOutputStream to implement all of nsIInputStream? Before I added nsISeekableOutputStream (which arguably should be named nsISeekableBufferedOutputStream, or even the unbearable nsIBackwardsSeekableBufferedOutputStream :-), the code in netwerk/base/src/ nsBufferedStreams.cpp that implements nsBufferedOutputStream::Fill was a no-op, which is just broken: if you seek back in such a stream and miss the buffer, then write less than a full buffer's worth of data, then seek again out of that buffer, or close, you'll write garbage. Ok, that's recap. As for why I renamed fill read, see my comment in attachment 45894 [details] [diff] [review]: - nsISeekableOutputStream inherits from nsISeekableStream, to reduce vtbl and QI overhead. This does require three forwards from nsBufferedOutputStream to nsBufferedStream superclass methods. Also, nsISeekableOutputStream.fill was not primitive, in light of seek, so it's now read -- which is exactly what nsFastLoadFileWriter checksumming code wants on the unbuffered stream. Hmm, that was terse. What I mean by "not primitive" is that fill was read+seek-back-to-buffer-start-offset, whereas seek was already exposed as a primitive in the inherited-from-nsISeekableStream nsISeekableOutputStream interface. It seems better to me to make interface methods orthogonal and primitive, unless there is a compelling overhead reduction (common sub-method elimination) in composing primitives, or perhaps if there is a common composition that everyone uses frequently (but the latter should be done generically, not by each interface impl, say via a "NS_ComposeAplusBHelper" inline or extern function, or perhaps service method, that calls A then B). So having decomposed nsISeekableOutputStreams's inherited and direct methods into primitives (setEOF still seems out of place to me, btw :-), I then saw that I could use read to compute the checksum in the nsFastLoadFileWriter case. This was serendipity, or a hack, depending on your taste. A more elaborate alternative would be to use nsIStreamIO or nsIFileIO, but (a) FastLoad is in xpcom, not netwerk; (b) it seemed overkill again, although not as much overkill as making the stream under a buffered, seekable output stream also implement nsIInputStream. /be

Here comes a patch that removes nsISeekableOutputStream, which turned out to be unnecessary once I had fixed nsBufferedOutputStream::mFillPoint to be a high watermark in the buffer, rather than a copy of mBufferSize (I fixed that while on the FASTLOAD_20010703_BRANCH, but failed to retest or otherwise realize that the fix relieved the need for nsISeekableOutputStream; before I changed what mFillPoint meant, a read-modify-write on the buffer was necessary or garbage would be written). Thanks to darin for setting me straight. The files that changed are content/xul/document/src/nsXULDocument.cpp netwerk/base/src/nsBufferedStreams.cpp netwerk/base/src/nsBufferedStreams.h netwerk/base/src/nsFileStreams.cpp netwerk/base/src/nsFileStreams.h xpcom/io/nsFastLoadFile.cpp xpcom/io/nsFastLoadFile.h xpcom/io/nsFastLoadService.cpp xpcom/io/nsISeekableStream.idl Still looking for r= and sr= love! /be /be

I knew I should have waited for my build to finish. If you apply the last diff -u patch, please apply this further patch: +++ ./nsFastLoadFile.cpp Thu Aug 16 20:44:12 2001 @@ -1705,7 +1705,7 @@ // XXXbe seek back to eof to force a flush -- why is there no interface for // flushing a buffered output stream? - rv = seekable->Seek(nsISeekableStream::NS_SEEK_SET, 0); + rv = seekable->Seek(nsISeekableStream::NS_SEEK_END, 0); if (NS_FAILED(rv)) return rv; // Now compute the checksum, using mFileIO to get an input stream on the There's no way to force a flush on a buffered stream without seeking far from the current offset. The - line above worked in the previous patch, where I'd deoptimized nsBufferedStream::Seek to dump its buffer when seeking below the cursor, but still within the buffer. There was no good reason for that deoptimization once I fixed nsBufferedOutputStream::Flush to use mFillPoint as the high-watermark when writing the buffer. Sorry, and I hope this does the trick! /be

r/sr=waterson

New patch coming up, I screwed up and failed to fix the mFillPoint high-watermark maintenance in nsBufferedOutputStream::Write. /be

Ok, I've tested twice, cut once this time. The changes from the last patch are to these files: - xpcom/io/nsFastLoadFile.cpp Revised to use nsIStreamBufferAccess to flush the header, rather than seek to EOF; also write the checksum directly, via the unbuffered output stream. This required nsBufferedOutputStream to implement nsIStreamBufferAccess. - netwerk/base/src/nsBufferedStreams.{h,cpp} Fixed mFillPoint maintenance: it was not being updated before each Fill call in the Write loop, only after the loop. But Fill has for the last few patches used mFillPoint as the high-watermark in the buffer through which to write. Also, nsBufferedOutputStream implements nsISBA. - netwerk/base/src/nsStdURL.cpp shaver caught a good of mine, assuing IsDirectory to be infallible. I'm gonna land soon, having got shaver and waterson blessings (shaver via IRC). /be

Tired, but I hope anyone who is following this nonsense knows that I meant "Flush()", not "Fill" in the blurb for nsBufferedStreams.{h,cpp}'s changes. Thanks again to darin, dbaron, shaver, waterson, and esp. jrgm, who needs to name his favorite comestible -- I'm buying. THe patch just went in. /be

Fixing database corruption caused by bug 95743. Pay no attention to the man behind the curtain.

Bother. Is it just gcc-2.96 optimized builds that crash with this patch? See bug 95785, fixed by backing me out.

My 2.96 build was optimized without debug but with debugging symbols and I wasn't able to start the browser. Also, if I remember correctly HP wouldn't start up either.

No, it was egcs 1.1.2, HP, gcc 2.95, and gcc 2.96 (but not gcc 3.0, Windows, or Mac).

Problem seems to be uninitialized variable (rv) at nsChromeProtocolHandler::NewURI() If i remove line: if (NS_FAILED(rv)) return rv; build starts ok.

Yep, the do_CreateInstance should have |&rv| as the optional second parameter. That would explain this perfectly.

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

Please note the comment in bug 95888.

Tomi, I could kiss you! Er, perhaps not -- but let me know your address and name your favorite fine aged wine, beer, tequila, or whatever beverage you like. New patch coming up today. /be

brendan: looks like you are really close. can you make it for 0.9.4? :-)

I've run through about every dialog I could find, with a build on win2k that had the latest fast load changes. I went through both with fast load enabled and with it (default) disabled. Both covered navigator and dialogs, composer and dialogs, mail and news, msg compose, bookmars, history, js console, preferences and subdialogs, and account/profile wizards usage, etc. I didn't encounter any problems with normal use (disabled) due to the fastload changes. There were however a couple of things to note: 1) the fastload enabled profile stopped updating the .mfl file after a while. I don't know when this started happening. 2) there were problems with various popups in the trunk build I had, but those same problems already existed in a trunk build from the same time that didn't have the fastload changes.

> I didn't encounter any problems with normal use (disabled) due to the > fastload changes I should have said also that I didn't encounter any problems when it was enabled either, aside from having it stopped serializing. However, when it was in that state, the apps all appeared to work fine (although I don't know whether it was de-serializing from .mfl at that point).

So. I am stupid. In the course of looking at the popups issue noted above, I happened to run the current trunk with the same profile with the XUL.mfl. This trunk build will honour that pref, but in a broken way. I think that is what horked the serialization file, and is a complete false alarm. As for the popup issues, a clobber cleared them up, in either the current trunk, or current trunk plus latest patch.

Tested optimized and debug on rh7.1, gcc2.96. All good, checked in. Still you must set the "nglayout.debug.disable_xul_fastload" pref to false to enable, and if you want to measure performance gains, please also measure with the new pref "nglayout.debug.checksum_xul_fastload_file" set to false, as well as set to or left at its true default. jrgm, the FastLoad file will reach a maximum size for a given set of XUL scripts loaded by chrome URIs. Maybe you hit that point? How big was the file? /be

The XUL.mfl file was 1213KB (1,241,119 bytes) at the point that it stopped growing. But I assume that it was my error that caused some undefined state to be reached, and this is not that hard upper limit.

/me should read prior comments before commenting. jrgm: I suspect running two builds sharing the same profile could lead to a FastLoad file that's not updated, but I'm not certain. Let me noodle on that, and let me know if it happens again. I don't expect crashes or hangs from such races, although the whole area of multiple instances sharing a profile is rife with bugs, I'm sure. Everyone who can help, please update from the trunk, flip the pref, and test the heck out of FastLoad. With enough testimony of only good effects, maybe we can turn this on for 0.9.4. /be

I'm using 2001082109 , and I'm seeing crashes with Fastload enabled. For example, when I load MailNews TB34337815M TB34337782W If I disable Fastload, it works fine.

WD, we've been here before. Until I checked in just within the last hour, FastLoad was not in working order on the trunk. Either pull and build your own browser, or wait till tomorrow's builds are available, before crying wolf. /be

For reference, here are some sizes for the serialized file after using major parts of the app: + navigator + bkmk sidebar == 488KB + prefs + all prefs panels + all pref subdialogs == 918KB + all other dialogs directly reachable from nav (including bookmarks, history, form manager, js console, help viewer) and XUL sidebars (history/bookmarks/search) == 1151KB + all mailnews, including wizards, news/mail subscribe, search, addressbook, msg compose, account properties, etc. == 2338KB + chatzilla == 2632KB + composer and all dialogs == 2855KB (does not account for security UI or NS AIM, and may have missed some dialogs [almost certainly missing some]. Also, some UI is shared in many places, so later counts may underestimate the count they would register on their own). ------------------------------ I also tried corrupting the XUL.mfl (by cutting a couple thousand characters out of the middle of the file). At that point, instead of chucking the file, it kept it in place and `touch' the file every time I opened a dialog (but added no further bytes to the file). This is on win2k, and I had not set any pref for nglayout.debug.checksum_xul_fastload_file (i.e., checksum was on).

Wow. From what jrgm reports I can only conclude that nsIFile is implemented differently (is broken) on win2k. If the checksum doesn't match, the file is deleted using nsIFile::Remove. At least, that's what nsXULDocument.cpp plainly does, and that's what I see happen with my optimized Linux build -- I used emacs to munge the file good and proper, then ran mozilla and the file disappeared. Anyone else running the patch or (since the checkin) an updated build, please sound off with your own results. The FastLoad file size could be shrunk a bit, as strings shows (it discloses numerous repeated strings). The JS engine keeps identifiers uniquely interned, but when serializing scripts, there is not yet any external atom table. That should be done; I'll get on it. /be

Weird... on my linux build, I altered XUL.mfasl (in Vim :) ) and ran it again. At first I thought that the file had disappeared, until I noticed that it had been renamed Invalid.mfasl.

Actually, that's the expected behaviour in a debug build (move it aside to that filename). In optimized, though, it should nuke it.

So this works correctly all the way to the point in nsIFile::Remove() where it is going to delete the file with |int remove(const char *path)|. I think it fails because the file is still open and the docs say the file must be closed before remove can be called. I'll file a bug.

Just trying fastload with 08/22 nightly on WinNT. I created a fresh profile for testing. Mozilla starts fine without fastload, if I turn it on it creates a 277kb large XUF.mfl and stalls. No window at all appears (after choosing the test profile in the Profilemanager window).

OK, forget the fubar I posted. Creating a new profile on an *updated* build was just not working. After *completely* deinstalling and reinstalling Mozilla "fastload" also works on Win like a charm. Sorry.

I think bug 96388 is no longer a dependency: it calls for a doc-comment fix to nsIFile.idl, but the FastLoad code must take care not to call nsIFile::Remove on a file to which there are open streams. Patch to nsXULDocument.cpp that takes such care coming right up. /be

I've tried re-enabling my pref for fastload and it's not creating the XUL.mfl file. This is with 2001082403. Yesterday's build had the same problem.

OK, please disregard my last comments. I'm an idiot! :)

a=asa for the last patch for fastload testers on mac and windows (not enabled by defualt)

attachment 46840 [details] [diff] [review] is in. Anyone witha clue on the Mac i/o (seek?) problems that sometimes cause a runaway FastLoad file, please jump in over at bug 95888. /be

I forgot to mention one nit with the last patch. WIth that checked in, a corrupt file is cleanly removed on windows now (need to check Mac), but when it goes to create a new file, it does not set the flags to create it. See http://lxr.mozilla.org/seamonkey/source/content/xul/document/src/nsXULDocument.cpp#4442 and that mInputStream is non-null (how come?). All this means is that in the event of a bogus/corrupt file, it will be removed but fastload will not be used (not serialized to disk) until the next time the browser is started. Good enough for now; wants a fix someday.

Please excuse this spam...but how does a user set a pref so that it persists past the next install of a nightly? Platform is win2k. I added the pref to prefs.js (in my moz profile) but that didn't live past today's moz install. Thanks.

<QA Ignore> Burleigh, did you set this pref with Mozilla shut down? Hand editing the prefs.js must be done without mozilla running. (The file will be read on startup and rewritten at shutdown.) </QA Ignore>

sr=waterson

r=jrgm

This won't be on by default in 0.9.4, but it will in 0.9.5 (with XBL added to FastLoad, for greater speedup). I do intend to get bug 95888 fixed for 0.9.4, however, so that those who choose to enable FastLoad get good results. /be

As per discussion with jrgm, changing QA contact to him -

Comment on attachment 52847 [details] [diff] [review] turn on fastload sr=waterson

r=hyatt

Turned on. If you have troubles that seem to be caused by FastLoad, please open new bugs. /be

removing item for this bug from the release notes since the bug is marked fixed. If this is in error, please make a note in the release notes bug for the current milestone. As of today, this is bug 133795. In a ew weeks, it will be some other bug.

VERIFIED since fastload cached chrome has been present for over a year.