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)

defect

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?]
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
I am all tingly inside. :-)
Target Milestone: --- → mozilla0.9.9
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?
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.
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.
That is true. 'location' need not be a dll at all. I am sold on the hash idea.
Keywords: nsbeta1-
I do not want to attempt this until 48888 is resolved.
Depends on: 48888
I do not want to attempt this until 48888 is resolved.
Target Milestone: mozilla0.9.9 → mozilla1.0
48888 is 1.1 bound.
Target Milestone: mozilla1.0 → mozilla1.1
As for comment #1 - Bug #111296 seems to be that potential leak of location.
bug 130381 would fix the leak in a wierd way - by arena allocating it.
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.