558613 - Add win7 only performance counter for "Modified Page Bytes Count"

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Rob Arnold [:robarnold]]

In debugging the memory leak in bug 558557, none of the memory metrics available in the task manager were able to detect the leak. However using perfmon, I was able to monitor the leak via the "Modified Page List Bytes" counter (thanks to bug 558557 comment 15 for pointing out where the memory was being charged). This does not directly measure vram (we have not been able to find a way to do that).

I am not sure how stable this number is - we believe it measures the number of pages that are modified in RAM but not written out to disk so it would measure more than paged-out vram usage. And of course vram usage is dependent upon the sizes of windows and the amount of memory on the video card. I guess what I'm trying to say is, expect some instability in the numbers - it's the giant leaks we're really trying and able to detect with this metric.

Note: the OS says Windows 7 but I think this should be available on XP and Vista too.

Comment 1

[Aki Sasaki (not active)]

You're asking for another metric in Talos, correct?
Who's going to be writing this, and do they need access to standalone Talos?

Comment 2

[Rob Arnold [:robarnold]]

(In reply to comment #1)
> You're asking for another metric in Talos, correct?
Yes

> Who's going to be writing this,

I don't know.

> and do they need access to standalone Talos?

I'm not sure? I haven't worked with Talos for almost 3 years now.

Comment 3

[alice nodelman [:alice] [:anode]]

We can add the counters listed on:

http://technet.microsoft.com/en-us/library/cc780836%28WS.10%29.aspx

I don't see one for "Modified Page List Bytes" - is it possible that it goes by another name?  Does something there look like it could be a match?

Comment 4

[Rob Arnold [:robarnold]]

I did not see it on that list. According to:

http://msdn.microsoft.com/en-us/library/aa965225%28VS.85%29.aspx

it does not exist on Windows Server 2k3. This leak would not appear on there anyways since 2k3 does not support Direct2D. I think this requires Server 2K8.

Comment 5

[alice nodelman [:alice] [:anode]]

These seems like something I could find out pretty quick on staging.

Comment 6

[alice nodelman [:alice] [:anode]]

Testing on staging talos-r3-xp-002.

Comment 7

[alice nodelman [:alice] [:anode]]

Modified Page List Bytes is also not supported on XP - we would need a counter to be supported on win7/winxp/vista for this to be easily rolled out.

Comment 8

[alice nodelman [:alice] [:anode]]

We are still interested in collecting Modified Page List Bytes so talos will have to be hacked to support different counter lists per-winxp/win7 instead of just per-linux/mac/win.

Comment 9

[alice nodelman [:alice] [:anode]]

Currently testing a patch on talos-staging-master02 to attempt to split out w7 counters from the standard winxp/vista counters.

Comment 10

[alice nodelman [:alice] [:anode]]

Attachment: add Modified Page List Bytes counter to win7 only

Separates out talos windows counters into two sets, one for winxp/vista and one for win7.

Comment 11

[alice nodelman [:alice] [:anode]]

So, now I'm not convinced that Modified Page Bytes Count is supported on win7.  I believe it to be available on Windows Server 2008 and Windows Vista only.  I'm going to poke around a bit more, but I have this running in stage and it is unable to collect data for that counter on a win7 test box.

Comment 12

[alice nodelman [:alice] [:anode]]

Okay, I had to hunt a bit - but 'Modified Page List Bytes' is not associated with any process.  It is a counter of the state of the memory on the machine itself.  Currently talos only monitors the state of a browser process and does not have the code to query the state of the general memory on the machine.

Is this still something that you are interested in?

Comment 13

[Rob Arnold [:robarnold]]

Yes, still interested but I suppose we'll need to see how much it naturally varies. Having this on Vista/Server 2008 would also be nice.

Comment 14

[alice nodelman [:alice] [:anode]]

We only do talos testing on winxp/win7.

It looks like I can get this going for win7 with some hacking (and some hope).

Comment 15

[alice nodelman [:alice] [:anode]]

Attachment: [checked in]add Modified Page List Bytes counter to win7 only (take 2)

jhammel wasn't comfortable reviewing the patches for this... catlee can you give it a try?  Otherwise I can dig somebody else up.

Comment 16

[alice nodelman [:alice] [:anode]]

Attachment: [checked in]add Modified Page List Bytes to graph server db

Comment 17

[alice nodelman [:alice] [:anode]]

Attachment: add Modified Page List Bytes to graph server db (sql only)

Comment 18

[alice nodelman [:alice] [:anode]]

sql applied to graphs-stage.

Comment 19

[Chris AtLee [:catlee]]

Comment on attachment 447693 [details] [diff] [review]
[checked in]add Modified Page List Bytes counter to win7 only (take 2)

>Index: ttest.py
>===================================================================
>RCS file: /cvsroot/mozilla/testing/performance/talos/ttest.py,v
>retrieving revision 1.43
>diff -u -8 -p -r1.43 ttest.py
>--- ttest.py	5 Mar 2010 16:02:48 -0000	1.43
>+++ ttest.py	27 May 2010 03:51:48 -0000
>@@ -86,18 +86,23 @@ class TTest(object):
>         self.cmanager = None
>         if self.remote == True:
>             self.platform_type = 'win_'
>         elif platform.system() == "Linux":
>             self.cmanager = __import__('cmanager_linux')
>             self.platform_type = 'linux_'
>             self._ffprocess = LinuxProcess()
>         elif platform.system() in ("Windows", "Microsoft"):
>+            if '5.1' in platform.version(): #winxp
>+              self.platform_type = 'win_'
>+            elif '6.1' in platform.version(): #w7
>+              self.platform_type = 'w7_'
>+            else:
>+              raise talosError('unsupported windows version')

Will we need to distinguish win7 64-bit here?


>@@ -130,18 +136,28 @@ class CounterManager:
>     for counter in self.registeredCounters:
>       path = win32pdh.MakeCounterPath((None, 'process', self.process,
>                                        None, -1, counter))
>       hq = win32pdh.OpenQuery()
>       try:
>         hc = win32pdh.AddCounter(hq, path)
>       except:
>         win32pdh.CloseQuery(hq)
>+        #assume that this is a memory counter for the system, not a process counter
>+        path = win32pdh.MakeCounterPath((None, 'Memory', None, None, -1 , counter))
>+        hq = win32pdh.OpenQuery()  
>+        try:                                                                       
>+          hc = win32pdh.AddCounter(hq, path)                                       
>+        except:                                                                    
>+          win32pdh.CloseQuery(hq)    

Not sure I follow this part.  Why is this happening inside the first except block?

Comment 20

[alice nodelman [:alice] [:anode]]

The first exempt will fail if it happens to be a 'Memory' counter instead of a counter associated with a process, it will then fall through to the except clause.

It is set up this way because the code doesn't know which are 'Memory' counters and which are 'Process' counters - so it let's it try it both ways before failing out.

Comment 21

[alice nodelman [:alice] [:anode]]

Comment on attachment 447694 [details] [diff] [review]
[checked in]add Modified Page List Bytes to graph server db

changeset:   302:a4cbcc091813

Comment 22

[alice nodelman [:alice] [:anode]]

Graph server db update applied to production.

mysql> insert into tests values (NULL,"tp4_modlistbytes","Tp4 (Modified Page List Bytes)",1,1,NULL);
Query OK, 1 row affected (0.03 sec)

mysql> insert into tests values (NULL,"tp4_modlistbytes_nochrome","Tp4 NoChrome (Modified Page List Bytes)",0,1,NULL);
Query OK, 1 row affected (0.01 sec)

Comment 23

[alice nodelman [:alice] [:anode]]

Comment on attachment 447693 [details] [diff] [review]
[checked in]add Modified Page List Bytes counter to win7 only (take 2)

Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.44; previous revision: 1.43
done
Checking in sample.config;
/cvsroot/mozilla/testing/performance/talos/sample.config,v  <--  sample.config
new revision: 1.45; previous revision: 1.44
done
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.64; previous revision: 1.63
done
Checking in cmanager_win32.py;
/cvsroot/mozilla/testing/performance/talos/cmanager_win32.py,v  <--  cmanager_win32.py
new revision: 1.11; previous revision: 1.10
done

Comment 24

[alice nodelman [:alice] [:anode]]

Now showing up on the waterfall:

tp4_modlistbytes: 71.7MB 

All done here.