15.40 KB, patch
|Details | Diff | Splinter Review|
847 bytes, patch
|Details | Diff | Splinter Review|
206 bytes, text/plain
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.
You're asking for another metric in Talos, correct? Who's going to be writing this, and do they need access to standalone Talos?
(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.
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?
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.
These seems like something I could find out pretty quick on staging.
Assignee: nobody → anodelman
Testing on staging talos-r3-xp-002.
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.
Summary: Add performance counter for "Modified Page Bytes Count" → Add win7 only performance counter for "Modified Page Bytes Count"
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.
Currently testing a patch on talos-staging-master02 to attempt to split out w7 counters from the standard winxp/vista counters.
Created attachment 447653 [details] [diff] [review] 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.
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.
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?
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.
We only do talos testing on winxp/win7. It looks like I can get this going for win7 with some hacking (and some hope).
Created attachment 447693 [details] [diff] [review] [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.
Created attachment 447694 [details] [diff] [review] [checked in]add Modified Page List Bytes to graph server db
Attachment #447694 - Flags: review?(catlee)
sql applied to graphs-stage.
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?
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 on attachment 447694 [details] [diff] [review] [checked in]add Modified Page List Bytes to graph server db changeset: 302:a4cbcc091813
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 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
Now showing up on the waterfall: tp4_modlistbytes: 71.7MB All done here.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.