Aaron P Reporter
	Description • 17 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6

When going to the about:memory page, Firefox would scan the current memory space and see how much memory various objects are using.

At the top would be a simple list of the various areas of memory that tend to eat up RAM. The ones that I can think of right now include plugin memory usage, page cache memory usage, image cache memory usage, and memory that has been freed but is still being reported by the OS as used. Any other categories that tend to use large amounts of memory should be listed here as well. Below this, there would be a total amount of memory actually being used by Firefox.

Optionally, a tree structure could be shown below this (like treesize) that would allow its sections to be expanded/contracted for more detail. This could be implemented in the future if it is deemed to difficult to initially address.

Someone mentioned that the about:cache page shows some of this information. Unfortunately, some of the information is a little bit mislabeled, so users commonly misinterpret it. There's also a bug in the display, so it displays an impossibly incorrect number after clearing the cache. Not surprisingly, the number of complaints about the bogus and misleading information (and about all memory complaints in general) has gone down drastically as memory leaks have been fixed. This suggests to me that people never look at this stuff if they are not having a problem.

If necessary, memory swapped to disk can be reported, but should be clearly labeled and in a separate section from the main RAM memory stats. For consistency, usage data in all sections should be reported in the same format (e.g. '25.1' for 25.1 MB used).

Obviously, some of these features are harder to implement than others. However, I think that once even a subset of this was implemented well, it could be used as an agent in addressing the various memory hog rumors that are hard to quash (while helping people understand why Firefox uses memory the way it does).

Reproducible: Always

	Jesse Ruderman
	Comment 1 • 17 years ago

I like the idea of giving advanced users an easy way to see how much of Firefox's memory use goes to plugins, http cache, bfcache, image cache, other allocations, malloc fragmentation, and malloc failing to return pages to the OS.  I wonder how feasible it is.

	Brendan Eich [:brendan]
	Comment 2 • 17 years ago

Taking...

/be

	pd
	Comment 3 • 17 years ago

would it be possible to 'name and shame' extensions that are poorly coded by highlighting any extension memory hogs on this about:memory page?

	Robert Accettura [:raccettura]
	Comment 4 • 17 years ago

(In reply to comment #3)
> would it be possible to 'name and shame' extensions that are poorly coded by
> highlighting any extension memory hogs on this about:memory page?
> 

This might be tough, since extensions aren't really that separate from the browser chrome.  Most extensions are just some extra javascript and an xul overlay.  I think figuring out what a particular extension consumes what could be pretty difficult to be accurate about.  At least in a way that would be interpretable by most people.

	Brendan Eich [:brendan]
	Comment 5 • 17 years ago

It will be possible if I get the trace-malloc stack walking to weave in the JS stack. Then allocations can be blamed on script filenames, which will identify (perhaps in obfuscated pathnames) extensions as well as built-in chrome.

/be

	Jesse Ruderman
	Comment 6 • 17 years ago

See also bug 393501, "Ability to dump the JS stack in tracerefcnt etc. stacks".

	Sylvain Pasche
	Comment 7 • 17 years ago

One suggestion: it would be nice to expose the various memory informations via APIs, for extensions to use. I can imagine extensions for displaying total memory usage in the status bar, displaying the memory usage over time on a canvas/svg graph, ...

about:memory would then consume those API's to display that information (this makes this bug depend on 349985).

some notes:
page cache memory usage: nsICacheDeviceInfo::totalSize (is this enough?)
XPCOM memory: bug 167035
heap size: see http://lxr.mozilla.org/mozilla/source/extensions/metrics/src/nsLoadCollector.cpp#143

	Jim Gettys
	Comment 8 • 17 years ago

Please don't forget support for the XRes extension on UNIX/Linux on the X Window System, since all the window system does is consume memory applications ask it to....  Moz/Firefox has been consuming one compressed copy of the data in the cache, and 2(!) uncompressed versions: one in the address space of Firefox and the other in X11.

	Sylvain Pasche
	Comment 9 • 17 years ago

I've been playing with this stuff recently. I did one component to expose all memory information centrally (mozIMemInfo) which is implemented in JS and asks the various subsystems for memory usage. There's another internal component for implementing things in C++.

Right now, it exposes allocated sizes of the following metrics:
* Total physical memory (using nspr)
* Virtual and resident size (could be moved to nspr, see bug 398683)
* malloced memory: this is available on Linux through the libc (mallinfo). I'm not sure if there's an equivalent on Windows or OS X for this.
* Memory cache device (corresponds to the "Storage in use" value displayed in about:cache). However, this value isn't looking like the sum of the allocated memory for the cached resources. I can see situations where that value is greater that the resident size, when loading a large image for instance. I didn't look into details about this yet.
* XPCOM allocated objects: this is kind of a hack where I'm inserting a counter inside NS_LogCtor/NS_LogDtor. This means you need to compile with refcount enabled for this to work. It is then exposed through a new interface in xpcom/base.

I only tested this on Linux for now, it should work on others except for the malloc thing. If you want to test the patch, you need to build with --enable-debug or --enable-logrefcnt. After build, do a "make MOZ_EXTENSIONS=meminfo" in your $objdir/extensions".

The XUL frontend is then available at chrome://meminfo/content/meminfo.xul. I'm using PlotKit for the canvas Pie and Bar rendering (screenshot coming).

	Sylvain Pasche
	Comment 10 • 17 years ago

it shows a session where I opened a bunch of tabs in a new window, and then closed it.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 11 • 17 years ago

Here's a start of infrastructure for this -- I've added hooks for the presshell framepools (based on shaver's patch from a while ago), compressed/uncompressed images, sqlite3, and memory cache (note that the memory cache number is bogus, due to the way the memory cache is used by imglib; the compressed/uncompressed image numbers are correct.  the mem cache number should probably be taken out entirely).

I need to add a way to get at the process numbers for the OS, but the above only accounts for 1MB out of the 45MB or so that firefox uses on startup with about:blank on win32.  Adding new memory reporters is fairly trivial, if we can find good places to do so.

	Jesse Ruderman
	Comment 12 • 17 years ago

Is this stuff (e.g. _sqlite_memused) going to be behind an ifdef, or will it always be included?

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 13 • 17 years ago

The idea is for all this stuff to always be included -- note that the sqlite thing is just a hack, because the latest version of sqlite has the sqlite3_used_memory() function already implemented.

	Sylvain Pasche
	Comment 14 • 17 years ago

> I need to add a way to get at the process numbers for the OS,

If you are thinking about things like Virtual/Resident memory size, the component in the sample extension I attached could easily be changed to use the new infrastructure. I took the memory reporting code from the metrics extension.

Another interesting number to report is the XPCOM objects allocated memory. I did some hacks for it the above sample (see comment 9). However, it didn't account for a lot of memory on startup last time I tried (a few megabytes).

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 15 • 17 years ago

This patch just removes the sqlite hacks, as they're not needed any more (we've gotten an upgraded sqlite).

This is a start of a simple memory-in-use reporting framework, allowing flexible registration of different reporters of in-use memory.  I've included a couple of sample ones here, including sqlite, decoded images, compressed images, and presshell framepool, and memory cache in-use/freelist.  I can split up the core bits from the addition of the reporters if necessary, but they're pretty separate already.

We'd want to get this in so that we can get some more meaningful reporting of in-use memory for users.  This doesn't actually provide any UI for viewing the data, that'd be done separately.  bsmedberg, if you'd rather I find someone else for review, let me know.

	Mike Schroepfer
	Comment 16 • 17 years ago

I'd like to get some version of this in for final - so moving to b+

	Mike Connor [:mconnor]
	Comment 17 • 17 years ago

Want this a whole lot, agree, but should be P3.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 18 • 17 years ago

Just the addition of the nsIMemoryReporterManager interface and implementation, split out from the previous patch.  No actual reporters.

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 19 • 17 years ago

Comment on attachment 306446 [details] [diff] [review]
just the xpcom memory reporter bits

r=shaver

	Benjamin Smedberg
	Comment 20 • 17 years ago

Comment on attachment 306446 [details] [diff] [review]
just the xpcom memory reporter bits

NS_MEMORY_REPORTER_MANAGER_CONTRACTID belongs in nsXPCOMCIDInternal.h... nsXPCOMCID.h is for frozen components only.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 21 • 17 years ago

Ah, will do, thanks!

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 22 • 17 years ago

Morphing this a bit; figure we should probably have separate bugs on each reporter.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 23 • 17 years ago

Checked in.  Additional things needed:

- memory reporters
- front end UI (either a xul page with privs, or ideally an extension)

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 24 • 17 years ago

Hrm, had to back this out:

mozilla/xpcom/base/nsIMemoryReporter.idl:41: can't open included file nsISimpleEnumerator.idl for reading

is there not a first export pass to put the idl files in the right place?  xpcom/base is built before xpcom/ds, so maybe nsISimpleEnumerator.idl isn't in place yet.  I can move the memory stuff to another xpcom subdir, but which one?

	Alex Vincent [:WeirdAl]
	Comment 25 • 17 years ago

vlad:  forward declare the nsISimpleEnumerator interface?

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 26 • 17 years ago

Heh, I just suggested the same thing on IRC.  Good thinking! :)

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 27 • 17 years ago

Yeah, I probably should've stopped to think instead of backing out.  Back in now, with forward declaration.

	Arthur
	Comment 28 • 17 years ago

Should the linux nightly 2008030904 already show something for about:memory? It doesn't for me.

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 29 • 17 years ago

See comment 23. This was only the back-end work.

	Boris Zbarsky [:bzbarsky]
	Comment 30 • 17 years ago

I assume we have no plans to enable memory use by anything with a non-ASCII description?  Say if an extension wants to report the memory it's using?

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 31 • 17 years ago

If you're trying to say "I think this API would be better if it supported wide strings, because I think these diagnostics will be less useful without them", then just say it?  Statements of position masquerading as questions are a slow path to resolution; less Socratic irony, more explicit requests and statements, please!

	Boris Zbarsky [:bzbarsky]
	Comment 32 • 17 years ago

If that's what I meant I would have said it.  But there are tradeoffs here in terms of C++ implementation annoyance in all the cases where we're likely to actually use this vs possible extendibility to cases like the one I mention in comment 30.  I haven't put in the thought to figure out what those tradeoffs are and which side the API should fall on, and I'm assuming that the patch author and reviewers have.  I did want to verify that assumption.  No Socratic method here, just a genuine question.

	Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond) Assignee
	Comment 33 • 17 years ago

Yeah, I'm thinking of this as fairly low-level API; the hassle of dealing with wide strings (and the chance of people ignoring a possible reporter spot just because the string goop is a pain) makes it not worth it.  Even if an extension does they can pick an ascii transliteration or something.

	Mike Shaver (:shaver -- probably not reading bugmail closely)
	Comment 34 • 17 years ago

(In reply to comment #32)
> No Socratic method here, just a genuine question.

Yeah, and I don't have historic reason to suspect you were doing otherwise, really.  Sorry, I was out of line.

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 35 • 16 years ago

Sadly there is no Firefox3.1a1 entry available within the Target Milestone list anymore. So setting it to Firefox 3.1 for now.

	neil@parkwaycc.co.uk
	Comment 36 • 14 years ago

Comment on attachment 306446 [details] [diff] [review]
just the xpcom memory reporter bits

>+#define NS_MEMORY_REPORTER_IMPLEMENT(_classname,_path,_desc,_usageFunction,_dataptr) \
>+    class MemoryReporter_##_classname : public nsIMemoryReporter {      \
>+    public:                                                             \
>+      NS_DECL_ISUPPORTS                                                 \
>+      NS_IMETHOD GetPath(char **memoryPath) { *memoryPath = strdup(_path); return NS_OK; } \
>+      NS_IMETHOD GetDescription(char **desc) { *desc = strdup(_desc); return NS_OK; } \
I don't know whether people are expected to be able to use this from outside of libxul, but I can only assume they can't, unless they take steps to link to jemalloc.