I guess I'm cool with this, but lets get some other people into the mix... adding brendan on the hopes that he adds other interested parties..
XPCOM/OJI plugins expect nsIMemory to be frozen.
Looks like dougt's patch doesn't change anything in the interface itself. That's good, given beard's comment. I never thought nsIMemory needed fixing (or needed renaming from nsIAllocator, for that matter -- warren killed himself and toasted the trees landing the change). The money bug is the one about avoiding going through nsIMemory or its impl's indirection all the time. Do we believe we're better off with this ability (not tested, probably broken) to field-upgrade our allocator? /be
That bug is: http://bugzilla.mozilla.org/show_bug.cgi?id=75620 if you get a nsIMemory, you may have to pay a penalty. The whole point of this interface is to ensure that you are using the same allocator in all of the places. We may be able to speed up the static helper class nsMemory (see http://bugzilla.mozilla.org/showattachment.cgi?attach_id=47375), but it can not be inlined for the same reasons above. The optimal solution is to use the system alloc in places where they do not have to pass buffers between interface.
doug, thanks for the bug cite (I commented there). I don't care whether OJI and plugins pay XPCOM costs, but I would bet we're already using a bunch of different direct hooks on malloc (mostly via NSPR and nsCRT) -- so who are we kidding about field-upgrades? malloc is malloc is malloc. Weak symbols help us on some OSes, should we want to wrap malloc. It's hard to bound access to memory in some parts of our system, such that we can be sure everyone who could get his hands on an allocation is within a cloud of code that doesn't expose that memory via XPCOM interfaces. I'm pretty sure we're paying a tax here for no benefit. Comments from others are more than welcome -- am I swerving too hard against this bit of COM purity? /be
Brendan, this bug is about freezing the nsIMemory interface which is required by embedders and plugin's. This bug is not about fixing users of nsIMemory to use raw malloc or another direct call, it is about making an interrface available that allows components to pass memory around without regard for allocator.
Comment on attachment 54585 [details] [diff] [review] Proposed Patch v.1 * A client that wishes to be notified of low memory situations (for * example, because the client maintains a large memory cache that * could be released when memory is tight) may register with the "should", not "may" - * observer service (see nsIObserverService) using the - * NS_MEMORY_PRESSURE_TOPIC ("memory-pressure") as the topic for + * observer service (see nsIObserverService) using the topic for * observation. "passing the <<relevant>> topic for observation." What should <<relevant>> be? If "memory-pressure", say so in this sentence. I think the sentence lost something crucial. + * Topics for Observeration Observation misspelled. I realize you just moved comments for the topic #defines (now gone) up into the big doc-comment, but how about defining terms like "the flusher"? Or juts change "flusher" to "pressure observer"? Fix these and firstname.lastname@example.org
Attachment #54585 - Flags: superreview+
fix checked in. Checking in content/xbl/src/nsXBLService.cpp; /cvsroot/mozilla/content/xbl/src/nsXBLService.cpp,v <-- nsXBLService.cpp new revision: 1.133; previous revision: 1.132 done Checking in intl/strres/src/nsStringBundle.cpp; /cvsroot/mozilla/intl/strres/src/nsStringBundle.cpp,v <-- nsStringBundle.cpp new revision: 1.112; previous revision: 1.111 done Checking in modules/libjar/nsJAR.cpp; /cvsroot/mozilla/modules/libjar/nsJAR.cpp,v <-- nsJAR.cpp new revision: 1.85; previous revision: 1.84 done Checking in xpcom/base/nsIAllocator.h; /cvsroot/mozilla/xpcom/base/nsIAllocator.h,v <-- nsIAllocator.h new revision: 1.15; previous revision: 1.14 done Checking in xpcom/base/nsIMemory.idl; /cvsroot/mozilla/xpcom/base/nsIMemory.idl,v <-- nsIMemory.idl new revision: 1.7; previous revision: 1.6 done Checking in xpcom/base/nsMemoryImpl.cpp; /cvsroot/mozilla/xpcom/base/nsMemoryImpl.cpp,v <-- nsMemoryImpl.cpp new revision: 1.17; previous revision: 1.16 done Checking in xpcom/base/nsMemoryImpl.h; /cvsroot/mozilla/xpcom/base/nsMemoryImpl.h,v <-- nsMemoryImpl.h new revision: 1.5; previous revision: 1.4 done
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.