127092 - Heap Analysis instrumentation in code

Status: RESOLVED FIXED

(SeaMonkey :: General, defect, P2)

Description

[Suresh Duddi (gone)]

Would be nice to have code in release builds to get heap statistics.

Comment 1

[Suresh Duddi (gone)]

Goals for instrumentation:
- Ultra light weight
- Non-interfering with any operation
- triggered explicitly

Comment 2

[Suresh Duddi (gone)]

Attachment: First draft at heap analysis

The code is enalbed by setting

set MOZ_CFLAGS=MOZ_HEAP_STATISTICS

This is a compile time switch. Output is analyzed by perl script
mozilla/tools/footprint/heapmap.pl

Comment 3

[Suresh Duddi (gone)]

Attachment: better ifdeffing

set MOZ_CFLAGS=-DMOZ_HEAP_STATISTICS
cd xpfe/appshell/src
make

Comment 4

[Suresh Duddi (gone)]

Attachment: Heapdump on MOZ_HeapDump message

Dumps heap to c:\heapdump.txt when app gets the message MOZ_HeapDump

Comment 5

[Suresh Duddi (gone)]

Attachment: Protecting agaist win98 (thanks Garrett)

Protects against use of HeapWalk() and GetProcessHeaps() from win98 where they
dont exist.

Comment 6

[Suresh Duddi (gone)]

Comment on attachment 70988 [details] [diff] [review]
Protecting agaist win98 (thanks Garrett)

r=blythe

Comment 7

[Suresh Duddi (gone)]

rpotts/brendan : sr=?

mozilla/tools/footprint :
  heapdump.cpp : sends the MOZ_HeapDump message to mozilla
  heapmap.pl   : perl script that analyzes the heap dump

I am hoping to get this into 0.9.9 USED heap and heap overhead would be the metrics

Comment 8

[Suresh Duddi (gone)]

Attachment: Disabling small heap allocator for debug builds

Small heap allocator isn't showing up in heapwalk. On win2k, non-debug CRT
(MSVCRT) doesn't use small heap. MSVCDRT (debug CRT) seems to be using it for
win2k. Disabling this altogether for debug builds only.

Effect:
- Accurate heap numbers on win2k debug builds
- Win98 uses it. But we dont have heapwalk() on win98. So no accurate
measurement. And release builds will not be affected.

Comment 9

[Suresh Duddi (gone)]

Comment on attachment 71001 [details] [diff] [review]
Disabling small heap allocator for debug builds

r=blythe

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 71001 [details] [diff] [review]
Disabling small heap allocator for debug builds

sr=[e-mail], but if rpotts wants to override me, I defer to him.

/be

Comment 11

[Suresh Duddi (gone)]

BTW, I have #if defined(DEBUG) && defined(XP_WIN32) around the #include
"malloc.h" in the second patch. 

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 70988 [details] [diff] [review]
Protecting agaist win98 (thanks Garrett)

sr=[e-mail], jwz would want you to spell out Mozilla, not use
MOZ_... -- write a letter!

/be

Comment 13

[rpotts (gone)]

looks good to me -- go for it!

-- rick

Comment 14

[Mike Shaver (:shaver emeritus)]

Comment on attachment 70988 [details] [diff] [review]
Protecting agaist win98 (thanks Garrett)

a=shaver for 0.9.9.

Comment 15

[Mike Shaver (:shaver emeritus)]

Comment on attachment 71001 [details] [diff] [review]
Disabling small heap allocator for debug builds

a=shaver for 0.9.9

Comment 16

[Suresh Duddi (gone)]

Attachment: Fine tuning the heapdump output

Marking previous two patches obsolete as they are approved and checked in.

This new patch is derived from my greated understanding of the windows
allocator. HEAPs have multiple REGIONS in them which have blocks that are
either USED, FREE or UNCOMMITTED (just Vm and hasnt been mapped into physical
memory or swap). Getting this detail printed into the heapdump file.

Will modify perl analysis script : mozilla/tools/footprint/heapmap.pl to parse
this new format.

Comment 17

[Suresh Duddi (gone)]

shaver: can you just approve the patch. This is just a print format change in
code that only gets called for heapdump which no user will every encounter.

Comment 18

[Suresh Duddi (gone)]

Comment on attachment 71689 [details] [diff] [review]
Fine tuning the heapdump output

r=dp

Comment 19

[Suresh Duddi (gone)]

Comment on attachment 71689 [details] [diff] [review]
Fine tuning the heapdump output

I meant r=blythe

Comment 20

[Suresh Duddi (gone)]

All instrumentation checked in.