Bug 392351 (aboutmemory)

implement about:memory framework core

RESOLVED FIXED in Firefox 3.5

Status

()

Firefox
General
P4
enhancement
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: Aaron P, Assigned: vlad)

Tracking

(Depends on: 2 bugs, Blocks: 2 bugs)

Trunk
Firefox 3.5
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

10 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

Comment 1

10 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.
Taking...

/be
Assignee: nobody → brendan
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

10 years ago
Alias: about:memory

Updated

10 years ago
Depends on: 391749

Updated

10 years ago
Depends on: 392009

Updated

10 years ago
Depends on: 392008
No longer depends on: 392009

Updated

10 years ago
Alias: about:memory → aboutmemory

Comment 3

10 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?
(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.
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
Status: NEW → ASSIGNED

Comment 6

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

Comment 7

10 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

Comment 8

10 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.

Comment 9

10 years ago
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

10 years ago
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.
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.
Assignee: brendan → vladimir

Comment 12

10 years ago
Is this stuff (e.g. _sqlite_memused) going to be behind an ifdef, or will it always be included?
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

10 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).

Updated

10 years ago
Priority: -- → P2

Updated

10 years ago
Flags: blocking-firefox3?
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.
Attachment #286735 - Attachment is obsolete: true
Attachment #298871 - Flags: review?(benjamin)

Comment 16

10 years ago
I'd like to get some version of this in for final - so moving to b+
Flags: blocking-firefox3? → blocking-firefox3+
Attachment #298871 - Flags: review?(benjamin) → review?(shaver)
Want this a whole lot, agree, but should be P3.
Priority: P2 → P3

Updated

10 years ago
Priority: P3 → P4
Attachment #298871 - Flags: review?(shaver)
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.
Attachment #306446 - Flags: review?(shaver)
Comment on attachment 306446 [details] [diff] [review]
just the xpcom memory reporter bits

r=shaver
Attachment #306446 - Flags: review?(shaver) → review+
Attachment #306446 - Flags: superreview?(benjamin)

Comment 20

10 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.
Attachment #306446 - Flags: superreview?(benjamin) → superreview+
Ah, will do, thanks!
Morphing this a bit; figure we should probably have separate bugs on each reporter.
Summary: Implement about:memory page → implement about:memory framework core
Checked in.  Additional things needed:

- memory reporters
- front end UI (either a xul page with privs, or ideally an extension)
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
vlad:  forward declare the nsISimpleEnumerator interface?
Heh, I just suggested the same thing on IRC.  Good thinking! :)
Yeah, I probably should've stopped to think instead of backing out.  Back in now, with forward declaration.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Comment 28

10 years ago
Should the linux nightly 2008030904 already show something for about:memory? It doesn't for me.
See comment 23. This was only the back-end work.
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?
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!
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.
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.
(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.

Updated

9 years ago
Blocks: 453420
Sadly there is no Firefox3.1a1 entry available within the Target Milestone list anymore. So setting it to Firefox 3.1 for now.
Target Milestone: --- → Firefox 3.1
Version: unspecified → Trunk
Blocks: 472209

Updated

9 years ago
Blocks: 269685
Blocks: 480841
Blocks: 480843
Target Milestone: Firefox 3.1 → Firefox 3.5
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.
You need to log in before you can comment on or make changes to this bug.