668137 - Add vsize to about:memory on Windows

Status: VERIFIED FIXED

(Toolkit :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

Per bug 664291, we can call GlobalMemoryStatusEx, which gives us

 * ullTotalVirtual - total amount of virtual memory available to a userspace process on this machine (usually 2G on 32-bit systems, but sometimes 3G)
 * ullAvailVirtual - amount of virtual address space the current process has left.

Subtracting these two values gives us vsize.

Comment 1

[Justin Lebar (not reading bugmail)]

> Per bug 664291

er, bug 664291 comment 32.

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

Comment 3

[Nicholas Nethercote [inactive]]

Comment on attachment 542935 [details] [diff] [review]
Patch v1

Review of attachment 542935 [details] [diff] [review]:
-----------------------------------------------------------------

Comment 4

[Phil Ringnalda (:philor)]

Backed out of -inbound in http://hg.mozilla.org/integration/mozilla-inbound/rev/6777320f6f29, OS X says

nsMemoryReporterManager.cpp
g++-4.2 -o nsMemoryReporterManager.o -c  -fvisibility=hidden -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DOSTYPE=\"Darwin9.2.0\" -DOSARCH=Darwin -D_IMPL_NS_COM -I/builds/slave/inbound-osx-dbg/build/xpcom/base/../build -I/builds/slave/inbound-osx-dbg/build/xpcom/base -I. -I../../dist/include -I../../dist/include/nsprpub  -I/builds/slave/inbound-osx-dbg/build/obj-firefox/dist/include/nspr -I/builds/slave/inbound-osx-dbg/build/obj-firefox/dist/include/nss       -fPIC  -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -isysroot /Developer/SDKs/MacOSX10.5.sdk -fno-strict-aliasing -fno-common -fshort-wchar -pthread -DNO_X11 -pipe  -DDEBUG -D_DEBUG -DTRACING -gdwarf-2   -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MF .deps/nsMemoryReporterManager.pp /builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp: In member function 'virtual nsresult nsMemoryReporterManager::Init()':
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp:418: error: expected type-specifier before 'MemoryReporter_Vsize'
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp:418: error: expected `)' before 'MemoryReporter_Vsize'
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp:418: error: no matching function for call to 'nsMemoryReporterManager::RegisterReporter(int*)'
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.h:37: note: candidates are: virtual nsresult nsMemoryReporterManager::RegisterReporter(nsIMemoryReporter*)
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.h: In constructor 'nsMemoryReporter::nsMemoryReporter(nsCString&, nsCString&, PRInt32, PRInt32, PRInt64, nsCString&)':
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.h:29: warning: 'nsMemoryReporter::mDesc' will be initialized after
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.h:28: warning:   'PRInt64 nsMemoryReporter::mAmount'
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp:517: warning:   when initialized here
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp: At global scope:
/builds/slave/inbound-osx-dbg/build/xpcom/base/nsMemoryReporterManager.cpp:118: warning: 'PRInt64 GetVsize()' defined but not used
make[6]: *** [nsMemoryReporterManager.o] Error 1

Comment 5

[Justin Lebar (not reading bugmail)]

Of course it doesn't compile on the one platform I didn't test.

This inbound thing works great.  :)

Comment 6

[Justin Lebar (not reading bugmail)]

It stuck this time on inbound (original bug was "MACOS" instead of "MACOSX").

http://hg.mozilla.org/integration/mozilla-inbound/rev/72b3e5161578

Comment 7

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/72b3e5161578

Comment 8

[James May [:fowl2]]

Comment on attachment 542935 [details] [diff] [review]
Patch v1

Review of attachment 542935 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/nsMemoryReporterManager.cpp
@@ +140,5 @@
> +  bool success = GlobalMemoryStatusEx(&s);
> +  if (!success)
> +    return -1;
> +
> +  return s.ullTotalPhys - s.ullAvailPhys;

Shouldn't this be using the *Virtual equivalents instead of the system wide physical stats? 

ie.

return s.ullTotalVirtual - s.ullAvailVirtual;

Comment 9

[Justin Lebar (not reading bugmail)]

> Shouldn't this be using the *Virtual equivalents instead of the system wide 
> physical stats? 

Hm, yes.  That's kind of embarrassing!

Comment 10

[Justin Lebar (not reading bugmail)]

Attachment: Fix

This has r=khuey over IRC.

Comment 11

[Justin Lebar (not reading bugmail)]

Followup landed on m-c, before the Aurora merge.  Thanks for pointing this out, James!

http://hg.mozilla.org/mozilla-central/rev/821b5076d2c0

Comment 12

[Mihaela Velimiroviciu (:mihaelav)]

Build identifier: Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0

Verified as fixed on WinXP: vsize was added to about:memory page.
Setting resolution to VERIFIED-FIXED.