Fastload landing increased size of principals structs

RESOLVED FIXED in mozilla0.9.4


18 years ago
15 years ago


(Reporter: bratell, Assigned: brendan)


Windows 2000

Firefox Tracking Flags

(Not tracked)



(2 attachments)



18 years ago
The leak figures on Tinderbox increased after the fastload landing:

nsXULPDGlobalObject                              56          -
nsSystemPrincipal                                56       7.69%
nsCodebasePrincipal                              64       6.67%
TOTAL                                           176
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

Comment 1

18 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)?

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

Comment 3

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

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.

Comment 6

18 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?



18 years ago
Priority: -- → P1
Target Milestone: --- → mozilla0.9.4

Comment 7

18 years ago
Probably a dup of bug 93146, although why did the principals structs bloat?

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

Comment 9

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

Last Resolved: 18 years ago
Resolution: --- → INVALID

Comment 10

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

Resolution: INVALID → ---


18 years ago

Comment 11

18 years ago
Created attachment 44959 [details] [diff] [review]
proposed fix: remove encode from JSPrincipals, make rt->principalsDecoder a transcoder

Comment 12

18 years ago
Created attachment 44960 [details] [diff] [review]
diff -wu version of patch

Comment 13

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

r=shaver, with the s/Xcode/Transcode/g change from IRC.

Comment 16

18 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

18 years ago
Never mind: shaver showed me.

Comment 18

18 years ago

Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.