490014 - memstats misleading when running profiler

Status: RESOLVED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Tommy Reilly]

The profiler uses quite a bit of memory that is included in private bytes so looking at the gross stats can be misleading.  Created bug to discuss solutions, ideas bandied about so far:

1) if stack trace logging is on print a warning before printing the gross stats

2) don't print the gross stats when the profiler is on

3) use a dedicated heap (HeapCreate/malloc zone) for profiler memory, tabulate that memory and subtract from private bytes.  How to get the raw number of pages for a Heap/malloc zone is TBD

4) get profiler memory out of private bytes by having its memory come from shared memory (pretty easy if the profiler used a private FixedMalloc/GCHeap instance and some new VMPI apis to use shared memory).  This wouldn't work on non-virtual memory systems.

Comment 1

[Rishit Shah]

I think that having gross stats from profiler is quite handy.  So not so sure about #1 and #2.

#3 and #4 are clean approaches but both the approaches suffer from the profiler using GCHashTable (With OPTION_MALLOC).  I would think that GCHashTable would need some tweaking in order to allocation memory for the profiler from a special zone.

Comment 2

[Rishit Shah]

Here are my thoughts - MemoryProfiler has the following allocations:

- Adding items to GCHashtable (VMPI_alloc)
- CategoryGroup and PackageGroup instances (VMPI_alloc() via GCAllocObject)
- String interning (VMPI_alloc)

5) Make MemoryProfiler allocations self-contained and keep a track of them internally.  This can be achieved by addressing and tracking the above as follows:
  - Remove OPTION_MALLOC and instead make method grow() virtual which is overloaded by a derived class that is used by the 3 Hashtable instances by the profiler.

  - CategoryGroup/PackageGroup can have their own versions of new/delete instead of deriving from GCAllocObject.  

  - Track allocations via VMPI_alloc() for string interning

It does involve some code overhead but IMO it makes the tracking centralized and self-contained.

Comment 3

[Tommy Reilly]

What does keep track internally mean?  To not affect gross stats you need to adjust the private bytes value by a number of pages, tracking individual allocations from the profiler isn't a complete answer.  I'm feeling drawn to #4 but w/o using shared memory just a seperate GCHeap/FixedMalloc instance that's deducted from memstats.

Comment 4

[Lars T Hansen]

Attachment: Expedient fix: disable printing + print warning if profiler enabled

The goal of this fix is to get this bug off the must-fix list for flash10.1 with a good-enough solution; we should revisit later.

Comment 5

[Rishit Shah]

Comment on attachment 402823 [details] [diff] [review]
Expedient fix: disable printing + print warning if profiler enabled

Even though the stats are misleading the broken down information does help quite a bit during memory profiling.

Disabling it is not recommended.

Comment 6

[Lars T Hansen]

Comment on attachment 402823 [details] [diff] [review]
Expedient fix: disable printing + print warning if profiler enabled

Let me try again:  Can I have a review of whether this patch disables the output correctly under the circumstances when the gross memory data are tainted by the presence of the memory profiler?  ie, did I miss something?

Comment 7

[Rishit Shah]

No.

Comment 8

[Lars T Hansen]

redux changeset:   2613:4e2c48439a19

Suggesting taking this off flash10.1 list now that misleading data are not being printed and accurate data are available under clearly understood circumstances.

Comment 9

[Tommy Reilly]

The answer to Lars questions is yes