nsIMemory needs to be frozen

RESOLVED FIXED in mozilla1.0



18 years ago
18 years ago


(Reporter: dougt, Assigned: dougt)


Windows 2000
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment)



18 years ago


18 years ago
Blocks: 98278


18 years ago
Target Milestone: --- → mozilla1.0

Comment 1

18 years ago

Comment 2

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

Comment 3

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


Comment 5

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

Comment 7

18 years ago
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 sr=brendan@mozilla.org
Attachment #54585 - Flags: superreview+

Comment 9

18 years ago
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
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
Checking in modules/libjar/nsJAR.cpp;
/cvsroot/mozilla/modules/libjar/nsJAR.cpp,v  <--  nsJAR.cpp
new revision: 1.85; previous revision: 1.84
Checking in xpcom/base/nsIAllocator.h;
/cvsroot/mozilla/xpcom/base/nsIAllocator.h,v  <--  nsIAllocator.h
new revision: 1.15; previous revision: 1.14
Checking in xpcom/base/nsIMemory.idl;
/cvsroot/mozilla/xpcom/base/nsIMemory.idl,v  <--  nsIMemory.idl
new revision: 1.7; previous revision: 1.6
Checking in xpcom/base/nsMemoryImpl.cpp;
/cvsroot/mozilla/xpcom/base/nsMemoryImpl.cpp,v  <--  nsMemoryImpl.cpp
new revision: 1.17; previous revision: 1.16
Checking in xpcom/base/nsMemoryImpl.h;
/cvsroot/mozilla/xpcom/base/nsMemoryImpl.h,v  <--  nsMemoryImpl.h
new revision: 1.5; previous revision: 1.4
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.