99151 - nsIMemory needs to be frozen

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Doug Turner (:dougt)]

Comment 1

[Doug Turner (:dougt)]

Attachment: Proposed Patch v.1

Comment 2

[Alec Flett]

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

[Patrick C. Beard]

XPCOM/OJI plugins expect nsIMemory to be frozen.

Comment 4

[Brendan Eich [:brendan]]

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

Comment 5

[Doug Turner (:dougt)]

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.

Comment 6

[Brendan Eich [:brendan]]

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

Comment 7

[Doug Turner (:dougt)]

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 8

[Brendan Eich [:brendan]]

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=[e-mail]

Comment 9

[Doug Turner (:dougt)]

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