Last Comment Bug 392351 - (aboutmemory) implement about:memory framework core
(aboutmemory)
: implement about:memory framework core
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: P4 enhancement with 21 votes (vote)
: Firefox 3.5
Assigned To: Vladimir Vukicevic [:vlad] [:vladv]
:
Mentors:
Depends on: 391749 392008
Blocks: 269685 480841 453420 472209 480843
  Show dependency treegraph
 
Reported: 2007-08-15 11:03 PDT by Aaron P
Modified: 2012-09-27 10:21 PDT (History)
85 users (show)
mtschrep: blocking‑firefox3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Sample extension (284.78 KB, patch)
2007-10-11 15:13 PDT, Sylvain Pasche
no flags Details | Diff | Splinter Review
frontend screenshot (73.50 KB, image/png)
2007-10-11 15:16 PDT, Sylvain Pasche
no flags Details
start of about:memory infrastructure (26.54 KB, patch)
2007-10-30 13:41 PDT, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Splinter Review
updated patch (23.51 KB, patch)
2008-01-23 21:45 PST, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Splinter Review
just the xpcom memory reporter bits (8.30 KB, patch)
2008-02-28 21:34 PST, Vladimir Vukicevic [:vlad] [:vladv]
shaver: review+
benjamin: superreview+
Details | Diff | Splinter Review

Description Aaron P 2007-08-15 11:03:39 PDT
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
Comment 1 Jesse Ruderman 2007-08-16 22:39:10 PDT
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.
Comment 2 Brendan Eich [:brendan] 2007-08-20 18:13:19 PDT
Taking...

/be
Comment 3 pd 2007-09-20 02:18:22 PDT
would it be possible to 'name and shame' extensions that are poorly coded by highlighting any extension memory hogs on this about:memory page?
Comment 4 Robert Accettura [:raccettura] 2007-09-20 05:33:53 PDT
(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.
Comment 5 Brendan Eich [:brendan] 2007-09-20 11:17:50 PDT
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
Comment 6 Jesse Ruderman 2007-09-22 20:16:35 PDT
See also bug 393501, "Ability to dump the JS stack in tracerefcnt etc. stacks".
Comment 7 Sylvain Pasche 2007-09-23 08:15:41 PDT
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
Comment 8 Jim Gettys 2007-09-24 12:04:58 PDT
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.
Comment 9 Sylvain Pasche 2007-10-11 15:13:07 PDT
Created attachment 284540 [details] [diff] [review]
Sample extension

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).
Comment 10 Sylvain Pasche 2007-10-11 15:16:01 PDT
Created attachment 284543 [details]
frontend screenshot

it shows a session where I opened a bunch of tabs in a new window, and then closed it.
Comment 11 Vladimir Vukicevic [:vlad] [:vladv] 2007-10-30 13:41:25 PDT
Created attachment 286735 [details] [diff] [review]
start of about:memory infrastructure

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.
Comment 12 Jesse Ruderman 2007-10-30 16:25:39 PDT
Is this stuff (e.g. _sqlite_memused) going to be behind an ifdef, or will it always be included?
Comment 13 Vladimir Vukicevic [:vlad] [:vladv] 2007-10-31 10:01:18 PDT
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.
Comment 14 Sylvain Pasche 2007-10-31 11:37:03 PDT
> 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).
Comment 15 Vladimir Vukicevic [:vlad] [:vladv] 2008-01-23 21:45:44 PST
Created attachment 298871 [details] [diff] [review]
updated patch

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.
Comment 16 Mike Schroepfer 2008-01-24 08:51:38 PST
I'd like to get some version of this in for final - so moving to b+
Comment 17 Mike Connor [:mconnor] 2008-02-20 16:59:09 PST
Want this a whole lot, agree, but should be P3.
Comment 18 Vladimir Vukicevic [:vlad] [:vladv] 2008-02-28 21:34:20 PST
Created attachment 306446 [details] [diff] [review]
just the xpcom memory reporter bits

Just the addition of the nsIMemoryReporterManager interface and implementation, split out from the previous patch.  No actual reporters.
Comment 19 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-02-29 05:38:47 PST
Comment on attachment 306446 [details] [diff] [review]
just the xpcom memory reporter bits

r=shaver
Comment 20 Benjamin Smedberg [:bsmedberg] 2008-03-05 11:13:34 PST
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.
Comment 21 Vladimir Vukicevic [:vlad] [:vladv] 2008-03-05 11:16:00 PST
Ah, will do, thanks!
Comment 22 Vladimir Vukicevic [:vlad] [:vladv] 2008-03-05 16:39:08 PST
Morphing this a bit; figure we should probably have separate bugs on each reporter.
Comment 23 Vladimir Vukicevic [:vlad] [:vladv] 2008-03-05 16:45:32 PST
Checked in.  Additional things needed:

- memory reporters
- front end UI (either a xul page with privs, or ideally an extension)
Comment 24 Vladimir Vukicevic [:vlad] [:vladv] 2008-03-05 16:57:26 PST
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?
Comment 25 Alex Vincent [:WeirdAl] 2008-03-05 17:02:41 PST
vlad:  forward declare the nsISimpleEnumerator interface?
Comment 26 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-03-05 17:04:52 PST
Heh, I just suggested the same thing on IRC.  Good thinking! :)
Comment 27 Vladimir Vukicevic [:vlad] [:vladv] 2008-03-05 17:44:34 PST
Yeah, I probably should've stopped to think instead of backing out.  Back in now, with forward declaration.
Comment 28 Arthur 2008-03-09 09:18:25 PDT
Should the linux nightly 2008030904 already show something for about:memory? It doesn't for me.
Comment 29 Henrik Skupin (:whimboo) 2008-03-09 09:28:05 PDT
See comment 23. This was only the back-end work.
Comment 30 Boris Zbarsky [:bz] 2008-03-09 10:41:40 PDT
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?
Comment 31 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-03-09 10:46:07 PDT
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!
Comment 32 Boris Zbarsky [:bz] 2008-03-09 11:06:06 PDT
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.
Comment 33 Vladimir Vukicevic [:vlad] [:vladv] 2008-03-09 12:20:38 PDT
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.
Comment 34 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-03-09 12:51:52 PDT
(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.
Comment 35 Henrik Skupin (:whimboo) 2008-09-03 23:47:23 PDT
Sadly there is no Firefox3.1a1 entry available within the Target Milestone list anymore. So setting it to Firefox 3.1 for now.
Comment 36 neil@parkwaycc.co.uk 2011-04-10 15:49:03 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.