496354 - Windows Mobile memory reporting through "GetPrivateBytes" not reporting System Heap memory usage

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Brent Getlin]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_6; en-us) AppleWebKit/525.27.1 (KHTML, like Gecko) Version/3.2.1 Safari/525.27.1
Build Identifier: Adobe(R) Flash(R) Player 10 (10,1,50,180)

Submit the following fixes:

1) Report WinMo "PrivateBytes" memory based on the currently active process slot (Slot 0) rather than the process slot assigned to the Flash Player.  This fixes an issue where we were not collecting memory allocated outside the GCHeap (one example being GCAllocObjects).

2) Remove looking for the "MEM_PRIVATE" flag as well, as we want to capture shared memory allocations that our player makes as well.  It might be POSSIBLE that other processes are allocating here but it seems better to overreport rather than under-report memory usage.

Reviewed by Dave George



Reproducible: Always

Steps to Reproduce:
1. Run the memory test file "CoverFlow.swf"
2. View the memory reported by the flash application
3. Compare to the memory actually being consumed by reviewing Windows CE 5.0 Platform Builder. 
4. Note the drastic difference
Actual Results:  
11404kb was reported

Expected Results:  
22856kb should be reported

Comment 1

[Brent Getlin]

Attachment: Memory Reporting Patch

Changelist number: 528123

1) Report WinMo "PrivateBytes" memory based on the currently active process slot (Slot 0) rather than the process slot assigned to the Flash Player.  This fixes an issue where we were not collecting memory allocated outside the GCHeap (one example being GCAllocObjects).

2) Remove looking for the "MEM_PRIVATE" flag as well, as we want to capture shared memory allocations that our player makes as well.  It might be POSSIBLE that other processes are allocating here but it seems better to overreport rather than under-report memory usage.

Comment 2

[Steven Johnson]

Comment on attachment 381545 [details] [diff] [review]
Memory Reporting Patch

The diff is a little scragged (what's up with the backslash at the end of each line?) but I get the idea.

Approving, but the ifdef'ing on the second change is kinda awkward (makes for hard-to-read code)... IMHO just ifdefing the whole if-else block would be clearer and less error prone.

Comment 3

[Steven Johnson]

Attachment: Revised patch

Previous patch would not apply because it relied on other, unsubmitted CE-specific changes (mainly to VMPI_getPrivateResidentPageCount) that were made in the Flash tree but never submitted to Tamarin. This patch includes those changes, plus a few minor formatting fixes.

Comment 4

[Lars T Hansen]

Erik, please make sure this landed and if so then close this bug.

Comment 5

[Erik Tierney]

Verified that the changes in this patch have already landed in TR through various other bugs.  Closing as fixed.