Closed
Bug 93043
Opened 23 years ago
Closed 23 years ago
Fastload landing increased size of principals structs
Categories
(SeaMonkey :: General, defect, P1)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla0.9.4
People
(Reporter: bratell, Assigned: brendan)
Details
Attachments
(2 files)
10.51 KB,
patch
|
Details | Diff | Splinter Review | |
9.55 KB,
patch
|
Details | Diff | Splinter Review |
The leak figures on Tinderbox increased after the fastload landing: --NEW-LEAKS-----------------------------------leaks------leaks%----------------------- nsXULPDGlobalObject 56 - nsSystemPrincipal 56 7.69% nsCodebasePrincipal 64 6.67% TOTAL 176 --NEW-BLOAT-----------------------------------bloat------bloat%----------------------- nsFastLoadService 36 - nsThreadPoolRunnable 32 100.00% nsXULPrototypeCache 188 30.56% nsThread 140 25.00% nsXULPrototypeDocument 1344 20.00% nsJARURI 5376 14.38% nsSystemPrincipal 56 7.69% nsCodebasePrincipal 1216 6.67% nsAggregatePrincipal 684 5.56% nsJSCID 3536 4.62% nsROCSSPrimitiveValue 5096 2.08% nsComputedDOMStyle 3200 2.04% XPCWrappedNativeProto 3192 1.79% xptiInterfaceInfo 103572 1.05% nsFactoryEntry 57816 0.63% nsGenericFactory 4420 0.45% nsHashtable 69608 0.06% nsCParserNode 861600 0.01% TOTAL 1121112
Assignee | ||
Comment 1•23 years ago
|
||
I bet the principals are from the XUL script cache. Since FastLoad is pref'd off by default, I am not sure why nsFastLoadService is leaking. Daniel, did you enable the pref (set nglayout.debug.disable_xul_fastload to false)? /be
No, it's just reporting that the size of the principals is 4 bytes bigger. The only new leak here is the nsXULPDGlobalObject. We shouldn't be leaking that, though...
Assignee | ||
Comment 3•23 years ago
|
||
I interposed nsISerializable between nsIPrincipal and nsISupports on each principal type's inheritance chain -- didn't that save another vtbl ptr? I did not add data members to any principals concrete class. /be
It's not getting GC'ed. The leaked GC roots at shutdown look like they're just the usual |nsXULPrototypeScript::mJSObject|s, although maybe they aren't usual and I've been looking at too many leaks.
So I can find it, fastload is bug 68045.
Assignee | ||
Comment 6•23 years ago
|
||
So if the XUL prototype cache survives past XPCOM shutdown, we'll leak the XULPDGlobalObject, and I bet that's now happening. But why? /be
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.4
Assignee | ||
Comment 7•23 years ago
|
||
Probably a dup of bug 93146, although why did the principals structs bloat? /be
Yes, the leaks were a dup of bug 93146, so retitling this bug.
Summary: Fastload landing increases tinderbox leaks → Fastload landing increased size of principals structs
Assignee | ||
Comment 9•23 years ago
|
||
I added an encode function pointer to JSPrincipals, and forgot. That is what made the nsBasePrincipal subclasses grow by four bytes. Closing as INVALID, not a bug (not enough bloat to fear). /be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → INVALID
Assignee | ||
Comment 10•23 years ago
|
||
I was mindlessly following the pattern established long ago of adding function pointers to JSPrincipals, but there is no opportunity to vary principals->encode among several functions, or have several principals each have its own function pointer different from the others. The principalsDecoder member of JSRuntime is another strong clue that only one serialization system at a time can use the engine's API. Patch coming up to recover the wasted space. BTW, the other function pointers in JSPrincipals never vary, and could be moved to a single "ops" struct (poor man's C vtbl) or to the JSRuntime. But given the age of those members, I'm concerned someone out there is using them, and may even be married to the JSPrincipals API. /be
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee | ||
Updated•23 years ago
|
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 11•23 years ago
|
||
Assignee | ||
Comment 12•23 years ago
|
||
Assignee | ||
Comment 13•23 years ago
|
||
Review craved. The diff -wu is easier to read, as in fusing nsEncodeJSPrincipals and nsDecodeJSPrincipals in caps/src/nsJSPrincipals.cpp, I had to indent the code more, and avoid early returns. /be
Comment 14•23 years ago
|
||
r/sr=jst
r=shaver, with the s/Xcode/Transcode/g change from IRC.
Comment 16•23 years ago
|
||
I guess I mentioned this on IRC last night but it must've been missed. I don't see where the space reduction is coming from in this patch?
Comment 17•23 years ago
|
||
Never mind: shaver showed me.
Assignee | ||
Comment 18•23 years ago
|
||
Fixed. /be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•