Closed
Bug 123660
Opened 23 years ago
Closed 23 years ago
nsFactoryEntry objects should share or eliminate 'location' string member
Categories
(Core :: XPCOM, defect, P3)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla1.1alpha
People
(Reporter: sfraser_bugs, Assigned: dougt)
References
Details
nsFactoryEntry::nsFactoryEntry calls nsCRT::strdup() 751 times on startup,
allocating a total of 21450 bytes. Many of these strings will be duplicates,
since each component library can have many factory entries. On average, there are
about 10 nsFactoryEntry objects per DLL (guessing). It seems that there would be
a win sharing those strings here (which would require some ownership fixing).
Alternatively, instead of using a string here, can we just point to the nsDll
object for that library and avoid any allocations?
[side note: nsFactoryEntry::ReInit() doesn't free 'location' before assigning
into it. Potential leak?]
Comment 1•23 years ago
|
||
Doug, if you want me to do this, let me know. I am sure you would be more than
excited to do this ;-)
Assignee: dp → dougt
Priority: -- → P3
Assignee | ||
Comment 3•23 years ago
|
||
The |location| variable in the nsFactoryEntry is need so that a client can not
unregister a factory-location without knowing its location. This is a kind of
safe guard similar to how one can not unregister a factory without know the
factory pointer.
Short of removing these checks, we could hash the location string into a
PRUint32 thus saving the heap allocation.
Does this make sense?
Comment 4•23 years ago
|
||
If we replace the location in nsFactoryEntry by say the nsDll pointer, the
safeguard would still be valid right - anyone unregistering will still need to
provide a valid location which would translate to a nsDll. We could double check
if the translations passed.
Assignee | ||
Comment 5•23 years ago
|
||
There is a boundry condition where the dll may not even be loaded and a client
wishes to unregister it. The new "dll pointer" member in nsFactoryEntry will be
null until the dll is actually loaded. so the pointer may not be available.
In any case, this approach is not good enough since it does not cover non native
loaders.
I think hashing the location into a 32 bit field gives us the bloat savings we
desire while preserving the the current functionality.
Comment 6•23 years ago
|
||
That is true. 'location' need not be a dll at all. I am sold on the hash idea.
Assignee | ||
Comment 7•23 years ago
|
||
I do not want to attempt this until 48888 is resolved.
Depends on: 48888
Assignee | ||
Comment 8•23 years ago
|
||
I do not want to attempt this until 48888 is resolved.
Assignee | ||
Updated•23 years ago
|
Target Milestone: mozilla0.9.9 → mozilla1.0
Comment 10•23 years ago
|
||
As for comment #1 - Bug #111296 seems to be that potential leak of location.
Comment 11•23 years ago
|
||
bug 130381 would fix the leak in a wierd way - by arena allocating it.
Assignee | ||
Comment 12•23 years ago
|
||
after finish 48888, i do not think that hashing the location is possible. Is
this really still a problem now that we allocate out of an arena? I am going to
mark this fixed until i am proved otherwise...
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•