Closed
Bug 68045
Opened 24 years ago
Closed 23 years ago
precompile chrome JS and load it incrementally ("fastload")
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
mozilla0.9.5
People
(Reporter: brendan, Assigned: brendan)
References
Details
(Keywords: memory-footprint, perf, topperf, Whiteboard: [nav+perf])
Attachments
(30 files)
Summary says it all. Code-level breakdown soon.
/be
Assignee | ||
Comment 1•24 years ago
|
||
Mostly startup perf win, lesser footprint savings (maybe half of 143K for first
browser window?), definitely mozilla0.9 timeframe.
/be
Severity: normal → major
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9
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?)
Assignee: brendan → shaver
Status: ASSIGNED → NEW
Comment 3•24 years ago
|
||
is the work for this on track?
Assignee | ||
Comment 4•24 years ago
|
||
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
Assignee: shaver → brendan
Assignee | ||
Updated•24 years ago
|
Keywords: mozilla0.9 → mozilla0.8.1
Target Milestone: mozilla0.9 → mozilla0.8.1
Assignee | ||
Comment 5•24 years ago
|
||
Assignee | ||
Comment 6•24 years ago
|
||
Assignee | ||
Comment 7•24 years ago
|
||
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
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•24 years ago
|
||
Assignee | ||
Comment 9•24 years ago
|
||
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.
Comment 11•24 years ago
|
||
r=rogerl. This also fixes a long open bug, #31003.
Comment 12•24 years ago
|
||
sr=hyatt
Assignee | ||
Comment 13•24 years ago
|
||
- 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
Assignee | ||
Comment 14•24 years ago
|
||
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
Assignee | ||
Comment 15•24 years ago
|
||
Assignee | ||
Comment 16•24 years ago
|
||
Assignee | ||
Comment 17•24 years ago
|
||
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
Comment 18•24 years ago
|
||
rest of this is for 0.9, correct?
Target Milestone: mozilla0.8.1 → mozilla0.9
Comment 20•24 years ago
|
||
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.
Assignee | ||
Comment 21•24 years ago
|
||
Not going to make 0.9, and not worth rushing.
/be
Keywords: mozilla0.9 → mozilla0.9.1
Target Milestone: mozilla0.9 → mozilla0.9.1
Assignee | ||
Comment 22•23 years ago
|
||
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
Keywords: mozilla0.9.1 → mozilla0.9.2
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Comment 23•23 years ago
|
||
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?
Comment 24•23 years ago
|
||
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...
Updated•23 years ago
|
Whiteboard: [nav+perf]
Assignee | ||
Comment 25•23 years ago
|
||
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
Updated•23 years ago
|
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Assignee | ||
Comment 26•23 years ago
|
||
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
Assignee | ||
Comment 27•23 years ago
|
||
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
Assignee | ||
Comment 28•23 years ago
|
||
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
Comment 29•23 years ago
|
||
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.
Assignee | ||
Comment 30•23 years ago
|
||
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
Comment 31•23 years ago
|
||
is there somewhere a linux or windows binary to download?
I would really like to see this branch :-)
Comment 32•23 years ago
|
||
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?
Assignee | ||
Comment 33•23 years ago
|
||
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
Comment 34•23 years ago
|
||
.idl files have to be added both to the MANIFIEST_IDL and the IDL.mcp project.
peterv fixed it.
Comment 35•23 years ago
|
||
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).
Comment 36•23 years ago
|
||
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.
Assignee | ||
Comment 37•23 years ago
|
||
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
Comment 38•23 years ago
|
||
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).
Assignee | ||
Comment 39•23 years ago
|
||
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
Assignee | ||
Comment 40•23 years ago
|
||
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
Comment 41•23 years ago
|
||
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.
Comment 42•23 years ago
|
||
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).
Assignee | ||
Comment 43•23 years ago
|
||
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
Comment 44•23 years ago
|
||
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.
Comment 45•23 years ago
|
||
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.
Assignee | ||
Comment 46•23 years ago
|
||
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
Comment 47•23 years ago
|
||
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].
Assignee | ||
Comment 48•23 years ago
|
||
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
Comment 49•23 years ago
|
||
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.
Assignee | ||
Comment 50•23 years ago
|
||
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
Comment 51•23 years ago
|
||
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. =)
Comment 52•23 years ago
|
||
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.
Assignee | ||
Comment 53•23 years ago
|
||
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!!!!!!!!!!!!!!!!")
Comment 54•23 years ago
|
||
Hey Kirk. Are you holding out on a change to nsFastLoadFile.h, like the
declaration of mSkipOffset?
Assignee | ||
Comment 55•23 years ago
|
||
(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
Comment 56•23 years ago
|
||
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
Comment 57•23 years ago
|
||
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.
Comment 58•23 years ago
|
||
Adam, you should probably update in content too.
Comment 59•23 years ago
|
||
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.
Assignee | ||
Comment 60•23 years ago
|
||
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
Comment 61•23 years ago
|
||
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.
Comment 62•23 years ago
|
||
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..?
Assignee | ||
Comment 63•23 years ago
|
||
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
Comment 64•23 years ago
|
||
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!'
Comment 65•23 years ago
|
||
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!
Assignee | ||
Comment 66•23 years ago
|
||
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
Comment 67•23 years ago
|
||
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.
Comment 68•23 years ago
|
||
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.
Assignee | ||
Comment 69•23 years ago
|
||
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
Assignee | ||
Comment 70•23 years ago
|
||
saari: sfraser saw 5-6% startup improvement on his debug build. We really need
to test optimized to get a good measurement.
/be
Comment 71•23 years ago
|
||
Do we know how smfr was measuring this? I'm guessing with the Apple profiling
tool...
Assignee | ||
Comment 72•23 years ago
|
||
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
Assignee | ||
Comment 73•23 years ago
|
||
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
Comment 74•23 years ago
|
||
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.
Comment 75•23 years ago
|
||
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.
Comment 76•23 years ago
|
||
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.
Assignee | ||
Comment 77•23 years ago
|
||
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
Assignee | ||
Comment 78•23 years ago
|
||
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
Assignee | ||
Comment 79•23 years ago
|
||
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
Assignee | ||
Comment 80•23 years ago
|
||
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
Assignee | ||
Comment 81•23 years ago
|
||
D'oh! I mean set nglayout.debug.disable_xul_fastload to false, to enable XUL
FastLoad. Darnit,
/be
Assignee | ||
Comment 82•23 years ago
|
||
Assignee | ||
Comment 83•23 years ago
|
||
Comment 84•23 years ago
|
||
Assignee | ||
Comment 85•23 years ago
|
||
Assignee | ||
Comment 86•23 years ago
|
||
I could use more reviews. The patch touches caps, netwerk, and mailnews, for
instance. Please help me land this by Tuesday night.
/be
Comment 87•23 years ago
|
||
mailnews changes look OK, assuming nothing bad is going to happen because those
methods are stubbed out.
Comment 88•23 years ago
|
||
Looks generally OK to me. Is there any risk of an attacker being able to use
principal de-serialization to forge a principal?
Assignee | ||
Comment 89•23 years ago
|
||
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
Comment 90•23 years ago
|
||
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?
Comment 91•23 years ago
|
||
What is the need for nsIStreamBufferAccess; doesn't ReadSegments work? (Heck, I
don't even see any callers in this patch);
Assignee | ||
Comment 92•23 years ago
|
||
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
Comment 93•23 years ago
|
||
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?
Comment 94•23 years ago
|
||
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.
Assignee | ||
Comment 95•23 years ago
|
||
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
Comment 96•23 years ago
|
||
cc'ing valeski b/c there may be other interfaces that are frozen or nearly frozen.
Comment 97•23 years ago
|
||
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?
Assignee | ||
Comment 98•23 years ago
|
||
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
Updated•23 years ago
|
Summary: precompile chrome JS and load it incrementally → precompile chrome JS and load it incrementally ("fastload")
Assignee | ||
Comment 99•23 years ago
|
||
Comment 100•23 years ago
|
||
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.
Comment 101•23 years ago
|
||
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 ?
Assignee | ||
Comment 102•23 years ago
|
||
Bernard: you need to apply the latest patch in this bug ("trunk-based patch
..."), and it sounds like you have not done that.
/be
Assignee | ||
Comment 103•23 years ago
|
||
Assignee | ||
Comment 104•23 years ago
|
||
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
Updated•23 years ago
|
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Assignee | ||
Comment 106•23 years ago
|
||
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
Comment 107•23 years ago
|
||
Adding to CC - sorry for the spam.
Comment 108•23 years ago
|
||
This bug has mozilla0.9.2 as a keyword. Update or remove?
Assignee | ||
Comment 109•23 years ago
|
||
Assignee | ||
Comment 110•23 years ago
|
||
Assignee | ||
Comment 111•23 years ago
|
||
Assignee | ||
Comment 112•23 years ago
|
||
Looking for dbaron r= and waterson/jst/hyatt sr=.
/be
Assignee | ||
Comment 113•23 years ago
|
||
I crave shaver's review, as always! bugmail spam gets him to give it up, usually.
/be
Assignee | ||
Comment 114•23 years ago
|
||
Assignee | ||
Comment 115•23 years ago
|
||
Assignee | ||
Comment 116•23 years ago
|
||
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
Comment 117•23 years ago
|
||
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?
Assignee | ||
Comment 118•23 years ago
|
||
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
Assignee | ||
Comment 119•23 years ago
|
||
Assignee | ||
Comment 120•23 years ago
|
||
Assignee | ||
Comment 121•23 years ago
|
||
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
Assignee | ||
Comment 122•23 years ago
|
||
Assignee | ||
Comment 123•23 years ago
|
||
Assignee | ||
Comment 124•23 years ago
|
||
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
Comment 125•23 years ago
|
||
r/sr=waterson
Assignee | ||
Comment 126•23 years ago
|
||
New patch coming up, I screwed up and failed to fix the mFillPoint
high-watermark maintenance in nsBufferedOutputStream::Write.
/be
assignee_accessible: 1 → 0
CC list accessible: false
qacontact_accessible: 1 → 0
Not accessible to reporter
Assignee | ||
Comment 127•23 years ago
|
||
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
Assignee | ||
Comment 128•23 years ago
|
||
Assignee | ||
Comment 129•23 years ago
|
||
Assignee | ||
Comment 130•23 years ago
|
||
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
Updated•23 years ago
|
assignee_accessible: 0 → 1
CC list accessible: true
qacontact_accessible: 0 → 1
Accessible to reporter
Comment 131•23 years ago
|
||
Fixing database corruption caused by bug 95743. Pay no attention to the man
behind the curtain.
Assignee | ||
Comment 132•23 years ago
|
||
Bother. Is it just gcc-2.96 optimized builds that crash with this patch? See
bug 95785, fixed by backing me out.
Comment 133•23 years ago
|
||
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.
Comment 134•23 years ago
|
||
No, it was egcs 1.1.2, HP, gcc 2.95, and gcc 2.96 (but not gcc 3.0, Windows, or
Mac).
Comment 135•23 years ago
|
||
Problem seems to be uninitialized variable (rv) at
nsChromeProtocolHandler::NewURI()
If i remove line:
if (NS_FAILED(rv)) return rv;
build starts ok.
Comment 136•23 years ago
|
||
Yep, the do_CreateInstance should have |&rv| as the optional second parameter.
That would explain this perfectly.
Comment 137•23 years ago
|
||
*** Bug 95888 has been marked as a duplicate of this bug. ***
Comment 138•23 years ago
|
||
Please note the comment in bug 95888.
Assignee | ||
Comment 139•23 years ago
|
||
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
Comment 140•23 years ago
|
||
brendan:
looks like you are really close. can you make it for 0.9.4? :-)
Assignee | ||
Comment 141•23 years ago
|
||
Assignee | ||
Comment 142•23 years ago
|
||
Comment 143•23 years ago
|
||
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.
Comment 144•23 years ago
|
||
> 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).
Comment 145•23 years ago
|
||
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.
Assignee | ||
Comment 146•23 years ago
|
||
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
Comment 147•23 years ago
|
||
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.
Assignee | ||
Comment 148•23 years ago
|
||
/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
Comment 149•23 years ago
|
||
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.
Assignee | ||
Comment 150•23 years ago
|
||
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
Comment 151•23 years ago
|
||
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).
Assignee | ||
Comment 152•23 years ago
|
||
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
Comment 153•23 years ago
|
||
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.
Comment 154•23 years ago
|
||
Actually, that's the expected behaviour in a debug build (move it aside to that
filename). In optimized, though, it should nuke it.
Comment 155•23 years ago
|
||
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.
Comment 156•23 years ago
|
||
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).
Comment 157•23 years ago
|
||
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.
Assignee | ||
Comment 158•23 years ago
|
||
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
Assignee | ||
Comment 159•23 years ago
|
||
Assignee | ||
Comment 160•23 years ago
|
||
Assignee | ||
Comment 161•23 years ago
|
||
Comment 162•23 years ago
|
||
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.
Comment 163•23 years ago
|
||
OK, please disregard my last comments. I'm an idiot! :)
Comment 164•23 years ago
|
||
a=asa for the last patch for fastload testers on mac and windows (not enabled by
defualt)
Assignee | ||
Comment 165•23 years ago
|
||
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
Comment 166•23 years ago
|
||
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.
Comment 167•23 years ago
|
||
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.
Comment 168•23 years ago
|
||
<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>
Assignee | ||
Comment 169•23 years ago
|
||
Comment 170•23 years ago
|
||
sr=waterson
Comment 171•23 years ago
|
||
r=jrgm
Assignee | ||
Comment 172•23 years ago
|
||
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
Keywords: mozilla0.9.2 → mozilla0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Comment 173•23 years ago
|
||
As per discussion with jrgm, changing QA contact to him -
QA Contact: pschwartau → jrgm
Assignee | ||
Comment 174•23 years ago
|
||
Comment 175•23 years ago
|
||
Comment on attachment 52847 [details] [diff] [review]
turn on fastload
sr=waterson
Attachment #52847 -
Flags: superreview+
Comment 176•23 years ago
|
||
r=hyatt
Assignee | ||
Comment 177•23 years ago
|
||
Turned on. If you have troubles that seem to be caused by FastLoad, please open
new bugs.
/be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 178•22 years ago
|
||
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.
Comment 179•22 years ago
|
||
VERIFIED since fastload cached chrome has been present for over a year.
Status: RESOLVED → VERIFIED
Updated•19 years ago
|
Flags: testcase-
Updated•19 years ago
|
Flags: testcase-
You need to log in
before you can comment on or make changes to this bug.
Description
•