Closed Bug 93043 Opened 23 years ago Closed 23 years ago

Fastload landing increased size of principals structs

Categories

(SeaMonkey :: General, defect, P1)

x86
Windows 2000
defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla0.9.4

People

(Reporter: bratell, Assigned: brendan)

Details

Attachments

(2 files)

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
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...
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 if the XUL prototype cache survives past XPCOM shutdown, we'll leak the
XULPDGlobalObject, and I bet that's now happening.  But why?

/be
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla0.9.4
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
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
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 → ---
Status: REOPENED → ASSIGNED
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
r/sr=jst
r=shaver, with the s/Xcode/Transcode/g change from IRC.
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?
Never mind: shaver showed me.
Fixed.

/be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: