Open Bug 519568 Opened 15 years ago Updated 2 years ago

Create new Talos suite: metrics for plugins

Categories

(Testing :: General, defect)

x86
All
defect

Tracking

(Not tracked)

People

(Reporter: benjamin, Unassigned)

References

Details

Attachments

(3 files, 3 obsolete files)

We really need some Talos metrics for plugins and OOP plugins. I can think of the following:

* TPlugLoad - having never loaded the test plugin, create an <embed> to load the test plugin. Time from when the embed is inserted until you get a notification that the plugin NPP_New is called.
* TPlugNew - after running TPlugLoad, do it again, but this time the plugin is already loaded so now you're just timing creating a new plugin instance
* plugin process memory usage - for OOPP only, measure how much memory is used by the plugin processes (using whatever memory metrics we use)
morphing summary a little to match other "new suite requests" for Talos, and make it easier to find.
OS: Mac OS X → All
Summary: Talos metrics for plugins → Create new Talos suite: metrics for plugins
I've been poking around at Talos code to try to figure out how to get Talos to report memory and cpu consumption for child processes.  Bsmedberg, can you clarify what you want:

- an aggregate of cpu/memory statistics for parent + child processes
- separate cpu/memory statistics for parent process + aggregate statistics for all child processes
- separate cpu/memory statistics for each process
I think the current statistics are mainly useful to identify the high-water mark, so aggregate CPU/memory for all processes.
This is my attempt to generate TPlugNew and TPlugCreate numbers... right now the numbers hover betwee 6 and 10ms, so I think I need a higher-resolution timer in order to generate more meaningful numbers. Blech.
Timers are here for POSIX, waiting on :luser r+.  Could try those patches for the time being.
Depends on: 522126
Attached patch cmanager_win32.py patch (obsolete) — Splinter Review
This patches cmanager_win32.py to include counter statistics from child processes.  A separate patch will be submitted for linux.

The counter manager now collects statistics for a group of processes, rather than just the primary firefox process.

Instead of tracking a single counter handle, this patch causes the counter manager to track a list of counter tuples.  The first item in each tuple is a counter handle, the second is a counter path.  Since new child processes can be launched at any point during a test, the counter paths are updated during each call to getCounterValue(), using win32pdh.ExpandCounterPaths().  If a new counter path is found which isn't in the list of tuples, it means a new child process has been launched.  In this case, the new counter path is added to the existing counter query, and the counter (handle, path) tuple is added to the counter list.

When getCounterValue() is called, it returns the sum of values for all counter paths in the list.  Invalid counter paths, which can occur when a process is terminated, are treated as having a value of '0'.

Tested successfully on Windows 7, e10s build, with a test that launches multiple child processes.  Also tested on an m-c build, to verify this has no ill effects when running a test without child processes.
Attachment #412697 - Flags: review?(anodelman)
Depends on: 529137
Driveby thinking, feel free to ignore if meaningless, but how well would any of this work if process ids were ever reused?
This patch would not be affected by reuse of process ids, since everything is managed using counter paths, which look something like:

\\process(firefox)\\% Processor Time #for the first instance
\\process(firefox#1)\\% Processor Time #for the second instance

The CounterManager on win32 is not aware of pids, but it is something to keep in mind for linux.
I have client-side code for TPlugLoad/New available here:
http://hg.mozilla.org/users/bsmedberg_mozilla.com/plugin-perftests/

It compiles separately from our tree so we can create binary versions to commit to the Talos tree. It prints:

__start_report158670__end_report

__start_report1691__end_report

__start_report1559__end_report

__start_report1606__end_report

__start_report1555__end_report

__start_report2315__end_report

__start_report1335__end_report

__start_report1368__end_report

__start_report1335__end_report

__start_report1430__end_report

Where the first number will be used for TPlugLoad and the rest for TPlugNew, and we'll have 10 runs or so for averaging TPlugLoad.
This patches the counter manager on both linux and win32 to collect aggregate statistics from all relevant processes:  the primary process + any number of child processes, which can be launched at arbitrary points during a test run.

This patch does not touch Mac code, and e10s is not supported on Mac.  Tested on Windows 7 and Ubuntu 9.04.

Regarding comment #7 and reuse of PIDs, this patch should work fine in the case that PIDs are reused (linux), since the PID list is rebuilt frequently from scratch.  On Windows it isn't a concern since PIDs are not used to track the processes there.
Attachment #412697 - Attachment is obsolete: true
Attachment #414411 - Flags: review?(anodelman)
Attachment #412697 - Flags: review?(anodelman)
Comment on attachment 414411 [details] [diff] [review]
counter manager patch (win32 + linux)

Should there also be cmanager_mac changes?  Or do the processes show up differently on mac?
There aren't any child processes on the Mac at all right now; e10s is supported only on Win and Linux.  At some future point when e10s is ported to the Mac, we'll have to make equivalent changes to cmanager_mac, but we don't need it right now.
No longer depends on: 522126
Comment on attachment 414411 [details] [diff] [review]
counter manager patch (win32 + linux)

Nit - I would like to see the files that you open get closed when you are done with them, eg:

+    maps = open(mapfile)
Attachment #414411 - Flags: review?(anodelman) → review-
Close files opened during linux statistics gathering.
Attachment #414411 - Attachment is obsolete: true
Attachment #418220 - Flags: review?(anodelman)
Comment on attachment 418220 [details] [diff] [review]
counter manager patch (win32 + linux), v 0.2

Now waiting on scheduling a downtime for landing.
Attachment #418220 - Flags: review?(anodelman) → review+
Comment on attachment 418220 [details] [diff] [review]
counter manager patch (win32 + linux), v 0.2

Actually applying the patch fails:

patching file cmanager_linux.py
patching file ffprocess_linux.py
patching file cmanager_win32.py
patch: **** malformed patch at line 394:  

Can I get a patch that applies cleanly?
Attachment #418220 - Flags: review+ → review-
Comment on attachment 418220 [details] [diff] [review]
counter manager patch (win32 + linux), v 0.2

Sorry, jumped the gun.  Just a final eol confusing things.
Attachment #418220 - Flags: review- → review+
Comment on attachment 418220 [details] [diff] [review]
counter manager patch (win32 + linux), v 0.2

Sorry for the spam, turned out the patch really was malformed and not all of it was being applied.  Please generate a new one that applies cleanly.
Attachment #418220 - Flags: review+ → review-
Sorry about; this patch applies cleanly.
Attachment #418220 - Attachment is obsolete: true
Attachment #418880 - Flags: review?(anodelman)
Attachment #418880 - Flags: review?(anodelman) → review+
this has a small bit of overlap with my patch in bug 474478, specifically in the ffprocess_linux.py changes.  I am not sure how best to do this with cvs.  Should I assume my patch will be applied after this?  

I am not a cvs master, so I don't know how to fix this patch so it will apply after mine and generate a new one with just those few updates.

Since this is on the schedule for 1/14/10, I would like to figure this out today so we don't have any surprises on the 14th and can get all these patches added to Talos.
uploading a version of the attachment that will apply cleanly after my patch from bug 474478.  I am assuming there is just the two patches.
The updated patch looks fine to me.  

> I am assuming there is just the two patches.

I think so, since the patch in bug 529137 has already landed.
Comment on attachment 421261 [details] [diff] [review]
[checked in]counter manager patch (win32 + linux), v OO

Checking in cmanager_linux.py;
/cvsroot/mozilla/testing/performance/talos/cmanager_linux.py,v  <--  cmanager_linux.py
new revision: 1.6; previous revision: 1.5
done
Checking in cmanager_win32.py;
/cvsroot/mozilla/testing/performance/talos/cmanager_win32.py,v  <--  cmanager_win32.py
new revision: 1.6; previous revision: 1.5
done
Checking in ffprocess_linux.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_linux.py,v  <--  ffprocess_linux.py
new revision: 1.16; previous revision: 1.15
done
Attachment #421261 - Attachment description: counter manager patch (win32 + linux), v OO → [checked in]counter manager patch (win32 + linux), v OO
Comment on attachment 421261 [details] [diff] [review]
[checked in]counter manager patch (win32 + linux), v OO

tp4 broken, backing this out to try and fix the issue.
Attachment #421261 - Attachment description: [checked in]counter manager patch (win32 + linux), v OO → [backed out]counter manager patch (win32 + linux), v OO
Comment on attachment 421261 [details] [diff] [review]
[checked in]counter manager patch (win32 + linux), v OO

Did not cause tp4 failure, checked back in.
Attachment #421261 - Attachment description: [backed out]counter manager patch (win32 + linux), v OO → [checked in]counter manager patch (win32 + linux), v OO
Component: New Frameworks → General
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: