Adding pyauto-based memory usage tests for ChromeOS.

Adding pyauto-based memory usage tests for ChromeOS.

These tests each open and close a set of tabs twice, and measure/record
memory usage information at certain points during test execution.

BUG=chromium-os:29815
TEST=Verified the test runs successfully on my local ChromeOS device running
a recent official build.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=134417

[dennis_jeffrey, 2012-04-25 18:32:31]

[Not ready for full review yet - just soliciting feedback].

Sending this out for initial high-level feedback from the code reviewers.  What
do you think of this approach for implementing the test?  More details below.

The test does the following:

1) Log into ChromeOS (first logs out if initially logged in)
2) Measure memory usage
3) Open 10 tabs to about:blank
4) Measure memory usage
5) Close the 10 tabs that were opened
6) Measure memory usage
7) Repeat steps 3-6 again

Notes:

* This is meant as a control test because the tabs opened are to about:blank. 
We can add another 1 or 2 tests that open a different set of sites, as described
in the bug associated with this CL.

* For some reason, restarting the session manager (as part of logout and login)
causes CPU usage to go from < 3% to 4 or 5%.  I had to increase the CPU usage
threshold from 3% to 5% to get this test to start up.

* I noticed that if I run this test multiple times without rebooting the device,
total system free memory recorded at the start of the test (after a fresh login)
gets gradually less and less over time.

[dennis_jeffrey, 2012-04-25 18:33:12]

On 2012/04/25 18:32:31, dennis_jeffrey wrote:
> [Not ready for full review yet - just soliciting feedback].
> 
> Sending this out for initial high-level feedback from the code reviewers. 
What
> do you think of this approach for implementing the test?  More details below.
> 
> The test does the following:
> 
> 1) Log into ChromeOS (first logs out if initially logged in)
> 2) Measure memory usage
> 3) Open 10 tabs to about:blank
> 4) Measure memory usage
> 5) Close the 10 tabs that were opened
> 6) Measure memory usage
> 7) Repeat steps 3-6 again
> 
> Notes:
> 
> * This is meant as a control test because the tabs opened are to about:blank. 
> We can add another 1 or 2 tests that open a different set of sites, as
described
> in the bug associated with this CL.
> 
> * For some reason, restarting the session manager (as part of logout and
login)
> causes CPU usage to go from < 3% to 4 or 5%.  I had to increase the CPU usage
> threshold from 3% to 5% to get this test to start up.
> 
> * I noticed that if I run this test multiple times without rebooting the
device,
> total system free memory recorded at the start of the test (after a fresh
login)
> gets gradually less and less over time.

By the way, here's an example of the perf keyvals written when this test runs:

_PERF_PRE_('KB_MemControl-GTT-Start', 40848.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-MemFree-Start', 3259556.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-GTT-Open1', 60288.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-MemFree-Open1', 3088592.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-GTT-Close1', 70412.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-MemFree-Close1', 3203640.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-GTT-Open2', 93128.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-MemFree-Open2', 3017332.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-GTT-Close2', 93308.000000)_PERF_POST_

_PERF_PRE_('KB_MemControl-MemFree-Close2', 3170648.000000)_PERF_POST_

[James Cook, 2012-04-25 19:15:56]

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1810: mem_free = re.search('MemFree:\s*([\d]+)
kB', stdout).group(1)
I recommend using available memory as a stat, which is what Luigi uses for the
low memory signal for the tab discarder.  AvailableMem = (ActiveFileMem +
InactiveFileMem) - DirtyMem + FreeMem.  You could record it in addition, or
instead of, this value.

We also got burned in the past with a Flash bug that was leaking "Shmem:", so
that might be good to track also.

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1827: self.AppendTab(pyauto.GURL(site))
Depending on how realistic you want this to be, versus the speed of the test,
you might want to sleep a second or two between loading each tab.  Also, does
this wait for the tab to finish loading before continuing?

[ilja, 2012-04-25 19:50:23]

I think it is a good start. We should still consider more carefully how we
measure which memory stat.

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:93: self._WaitForIdleCPU(60.0, 0.05)
Any reason you need to change this?

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1797: 
I spoke with Stephane about your numbers. He thinks you should pause 20 seconds
before taking a final measurement to allow the graphics driver to reclaim memory
(which will only happen when the system is under pressure or idle).

But he is also interested in peak numbers, which would imply continuous
sampling. Maybe like the WaitForIdleCPU we could sample these numbers once per
second, wait until things stabilize and analyze it as post process?

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1810: mem_free = re.search('MemFree:\s*([\d]+)
kB', stdout).group(1)
Lets track as James suggest AvailableMem instead of FreeMem as this is used in
the tab discard decision.

Tracking shared memory sounds like a good idea.

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1827: self.AppendTab(pyauto.GURL(site))
I think it does not. But maybe we don't need to wait here if we change
GetMemoryStats to sample once per second for at least 20 seconds (or until
numbers stabilize)?

[dennis_jeffrey, 2012-04-25 21:31:58]

Modified the code to record available memory (instead of free memory), and also
shared memory.

Have a question about sampling and waiting for numbers to stabilize before
reporting them.

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:93: self._WaitForIdleCPU(60.0, 0.05)
On 2012/04/25 19:50:23, ilja wrote:
> Any reason you need to change this?

Yes - I mentioned about it as one of the notes in the original review e-mail
that was sent out:

"* For some reason, restarting the session manager (as part of logout and login)
causes CPU usage to go from < 3% to 4 or 5%.  I had to increase the CPU usage
threshold from 3% to 5% to get this test to start up."

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1797: 
On 2012/04/25 19:50:23, ilja wrote:
> I spoke with Stephane about your numbers. He thinks you should pause 20
seconds
> before taking a final measurement to allow the graphics driver to reclaim
memory
> (which will only happen when the system is under pressure or idle).
> 
> But he is also interested in peak numbers, which would imply continuous
> sampling. Maybe like the WaitForIdleCPU we could sample these numbers once per
> second, wait until things stabilize and analyze it as post process?

I was confused at first about exactly when you wanted to do this sampling, but
in reading your comment at line 1827 below, it looks like you're suggesting that
every call to GetMemoryStats should sample until things stabilize (or up to 20
seconds).

So what do you recommend as a definition for when numbers "stabilize"?  Maybe if
each measured value does not change by more than 3% from the value measured in
the last second?  (and maybe we just give up and let the test proceed if nothing
stabilizes before 20 seconds?)

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1810: mem_free = re.search('MemFree:\s*([\d]+)
kB', stdout).group(1)
On 2012/04/25 19:50:23, ilja wrote:
> Lets track as James suggest AvailableMem instead of FreeMem as this is used in
> the tab discard decision.
> 
> Tracking shared memory sounds like a good idea.

Sure, we can track this data.  Puneet also said that he's interested in tracking
a wider variety of data, but for a start we can at least get available/shared
memory.

https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functio...
chrome/test/functional/perf.py:1827: self.AppendTab(pyauto.GURL(site))
On 2012/04/25 19:50:23, ilja wrote:
> I think it does not. But maybe we don't need to wait here if we change
> GetMemoryStats to sample once per second for at least 20 seconds (or until
> numbers stabilize)?

AppendTab should return only when notified of the
content::NOTIFICATION_LOAD_STOP event for the given tab.  Note that this doesn't
necessarily mean that all content on the tab has loaded though (e.g., in Gmail,
which uses ajax).

Tabs like those to about:blank open up nearly instantaneously, but you'll see
noticeable waits in-between each call to AppendTab if loading live sites like
Gmail, Plus, etc.

[dennisjeffrey, 2012-04-25 21:35:21]

I just saw this comment in the associated bug regarding sampling of data:

http://code.google.com/p/chromium-os/issues/detail?id=29815#c13

I'll make that change now and then upload a new patch set after that.

-Dennis

On Wed, Apr 25, 2012 at 2:31 PM, <dennisjeffrey@chromium.org> wrote:

> Modified the code to record available memory (instead of free memory), and
> also
> shared memory.
>
> Have a question about sampling and waiting for numbers to stabilize before
> reporting them.
>
>
>
> https://chromiumcodereview.**appspot.com/10161033/diff/**
>
2001/chrome/test/functional/**perf.py<https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functional/perf.py>
> File chrome/test/functional/perf.py (right):
>
> https://chromiumcodereview.**appspot.com/10161033/diff/**
>
2001/chrome/test/functional/**perf.py#newcode93<https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functional/perf.py#newcode93>
> chrome/test/functional/perf.**py:93: self._WaitForIdleCPU(60.0, 0.05)
> On 2012/04/25 19:50:23, ilja wrote:
>
>> Any reason you need to change this?
>>
>
> Yes - I mentioned about it as one of the notes in the original review
> e-mail that was sent out:
>
>
> "* For some reason, restarting the session manager (as part of logout
> and login)
> causes CPU usage to go from < 3% to 4 or 5%.  I had to increase the CPU
> usage
> threshold from 3% to 5% to get this test to start up."
>
> https://chromiumcodereview.**appspot.com/10161033/diff/**
>
2001/chrome/test/functional/**perf.py#newcode1797<https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functional/perf.py#newcode1797>
> chrome/test/functional/perf.**py:1797:
>
> On 2012/04/25 19:50:23, ilja wrote:
>
>> I spoke with Stephane about your numbers. He thinks you should pause
>>
> 20 seconds
>
>> before taking a final measurement to allow the graphics driver to
>>
> reclaim memory
>
>> (which will only happen when the system is under pressure or idle).
>>
>
>  But he is also interested in peak numbers, which would imply
>>
> continuous
>
>> sampling. Maybe like the WaitForIdleCPU we could sample these numbers
>>
> once per
>
>> second, wait until things stabilize and analyze it as post process?
>>
>
> I was confused at first about exactly when you wanted to do this
> sampling, but in reading your comment at line 1827 below, it looks like
> you're suggesting that every call to GetMemoryStats should sample until
> things stabilize (or up to 20 seconds).
>
> So what do you recommend as a definition for when numbers "stabilize"?
> Maybe if each measured value does not change by more than 3% from the
> value measured in the last second?  (and maybe we just give up and let
> the test proceed if nothing stabilizes before 20 seconds?)
>
>
> https://chromiumcodereview.**appspot.com/10161033/diff/**
>
2001/chrome/test/functional/**perf.py#newcode1810<https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functional/perf.py#newcode1810>
> chrome/test/functional/perf.**py:1810: mem_free =
> re.search('MemFree:\s*([\d]+) kB', stdout).group(1)
> On 2012/04/25 19:50:23, ilja wrote:
>
>> Lets track as James suggest AvailableMem instead of FreeMem as this is
>>
> used in
>
>> the tab discard decision.
>>
>
>  Tracking shared memory sounds like a good idea.
>>
>
> Sure, we can track this data.  Puneet also said that he's interested in
> tracking a wider variety of data, but for a start we can at least get
> available/shared memory.
>
>
> https://chromiumcodereview.**appspot.com/10161033/diff/**
>
2001/chrome/test/functional/**perf.py#newcode1827<https://chromiumcodereview.appspot.com/10161033/diff/2001/chrome/test/functional/perf.py#newcode1827>
> chrome/test/functional/perf.**py:1827: self.AppendTab(pyauto.GURL(**site))
> On 2012/04/25 19:50:23, ilja wrote:
>
>> I think it does not. But maybe we don't need to wait here if we change
>> GetMemoryStats to sample once per second for at least 20 seconds (or
>>
> until
>
>> numbers stabilize)?
>>
>
> AppendTab should return only when notified of the
> content::NOTIFICATION_LOAD_**STOP event for the given tab.  Note that this
> doesn't necessarily mean that all content on the tab has loaded though
> (e.g., in Gmail, which uses ajax).
>
> Tabs like those to about:blank open up nearly instantaneously, but
> you'll see noticeable waits in-between each call to AppendTab if loading
> live sites like Gmail, Plus, etc.
>
>
https://chromiumcodereview.**appspot.com/10161033/<https://chromiumcodereview...
>

[dennis_jeffrey, 2012-04-25 22:06:56]

Modified the code as per Ilja's comment in the associated bug to have
_GetMemoryStats() sample once per second up to some specified duration, then
output the max/min/ending values.

When running with a sampling duration of 10 seconds on each call to
_GetMemoryStats, here's an example of the perf keyvals outputted by the test (we
may choose to only plot a subset of these on the perf graphs):

_PERF_PRE_('KB_MemCtrl-GTTMin-Start', 41056.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMax-Start', 56100.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTEnd-Start', 41056.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMin-Start', 3327544.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMax-Start', 3352804.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailEnd-Start', 3349140.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMin-Start', 64380.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMax-Start', 82388.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedEnd-Start', 64384.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMin-Open1', 84848.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMax-Open1', 99448.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTEnd-Open1', 84848.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMin-Open1', 3099212.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMax-Open1', 3122592.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailEnd-Open1', 3114996.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMin-Open1', 144888.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMax-Open1', 163900.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedEnd-Open1', 149300.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMin-Close1', 45280.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMax-Close1', 84848.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTEnd-Close1', 45280.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMin-Close1', 3279612.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMax-Close1', 3324140.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailEnd-Close1', 3324008.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMin-Close1', 70708.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMax-Close1', 113236.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedEnd-Close1', 70708.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMin-Open2', 89072.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMax-Open2', 96496.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTEnd-Open2', 89072.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMin-Open2', 3097100.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMax-Open2', 3112728.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailEnd-Open2', 3104648.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMin-Open2', 153092.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMax-Open2', 160948.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedEnd-Open2', 153524.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMin-Close2', 49488.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTMax-Close2', 89072.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-GTTEnd-Close2', 49504.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMin-Close2', 3258360.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailMax-Close2', 3312276.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemAvailEnd-Close2', 3312276.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMin-Close2', 74932.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedMax-Close2', 118564.000000)_PERF_POST_

_PERF_PRE_('KB_MemCtrl-MemSharedEnd-Close2', 74932.000000)_PERF_POST_

[ilja, 2012-04-25 23:37:36]

I think the numbers you have posted are very interesting already. We are getting
very close. Just pipe duration to RunTest as I think it depends on which content
we will chose and rename the output values slightly and you are good from my
perspective.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1773: 
I assume you are logging out to use a fresh copy of Chrome and clean memory
state. Please leave a note to this intent.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1785: def _GetMemoryStats(self, duration=10):
I spoke with James and he thinks duration should be 15..20 seconds while
Stephane wanted 10..20. But see my comments below on loading pages. I think
better have no default.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1859: mem = self._GetMemoryStats()
Lets not use the default duration here. If we are loading 10 empty tabs 15
seconds might be appropriate. But if we are loading heavy websites maybe we want
to wait much longer (I think our test that opens gmail, G+ etc. measures 5
seconds or more per tab to load).

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1867: '%s-%sMin-%s' % (description, type_string,
when),
'%s-Min%s-%s' etc. is easier to read and does not fuse with GTT.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1870: '%s-%sMax-%s' % (description, type_string,
when),
dito

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1873: '%s-%sEnd-%s' % (description, type_string,
when),
dito

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1876: def _RunTest(self, tabs, description):
_RunTest needs duration as parameter when we run with real websites.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1877: self._RecordMemoryStats(description,
'Start')
Can we call this Close0 to be consistent with the rest? Or maybe 0Tabs0 which I
think is easiest to understand?

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1883: self._RecordMemoryStats(description,
'Open%d' % (iteration_num + 1))
'%dTabs%d' % (len(tabs), iteration_num + 1)

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1888: self._RecordMemoryStats(description,
'Close%d' % (iteration_num + 1))
'0Tabs%d'

[dennis_jeffrey, 2012-04-26 01:46:25]

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1773: 
On 2012/04/25 23:37:37, ilja wrote:
> I assume you are logging out to use a fresh copy of Chrome and clean memory
> state. Please leave a note to this intent.

Done.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1785: def _GetMemoryStats(self, duration=10):
On 2012/04/25 23:37:37, ilja wrote:
> I spoke with James and he thinks duration should be 15..20 seconds while
> Stephane wanted 10..20. But see my comments below on loading pages. I think
> better have no default.

I removed the default value to force the caller to choose an appropriate value.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1859: mem = self._GetMemoryStats()
On 2012/04/25 23:37:37, ilja wrote:
> Lets not use the default duration here. If we are loading 10 empty tabs 15
> seconds might be appropriate. But if we are loading heavy websites maybe we
want
> to wait much longer (I think our test that opens gmail, G+ etc. measures 5
> seconds or more per tab to load).

Now using 15 seconds as the duration for the empty tabs.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1867: '%s-%sMin-%s' % (description, type_string,
when),
On 2012/04/25 23:37:37, ilja wrote:
> '%s-Min%s-%s' etc. is easier to read and does not fuse with GTT.

Done.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1870: '%s-%sMax-%s' % (description, type_string,
when),
On 2012/04/25 23:37:37, ilja wrote:
> dito

Done.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1873: '%s-%sEnd-%s' % (description, type_string,
when),
On 2012/04/25 23:37:37, ilja wrote:
> dito

Done.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1876: def _RunTest(self, tabs, description):
On 2012/04/25 23:37:37, ilja wrote:
> _RunTest needs duration as parameter when we run with real websites.

Done.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1877: self._RecordMemoryStats(description,
'Start')
On 2012/04/25 23:37:37, ilja wrote:
> Can we call this Close0 to be consistent with the rest? Or maybe 0Tabs0 which
I
> think is easiest to understand?

I changed it to "0Tabs0" to follow the same format as what's recommended in your
comment at line 1883 below.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1883: self._RecordMemoryStats(description,
'Open%d' % (iteration_num + 1))
On 2012/04/25 23:37:37, ilja wrote:
> '%dTabs%d' % (len(tabs), iteration_num + 1)

Done.

https://chromiumcodereview.appspot.com/10161033/diff/5002/chrome/test/functio...
chrome/test/functional/perf.py:1888: self._RecordMemoryStats(description,
'Close%d' % (iteration_num + 1))
On 2012/04/25 23:37:37, ilja wrote:
> '0Tabs%d'

Done.

[ilja, 2012-04-26 02:44:35]

lgtm but lets wait for Sonny and James.

[James Cook, 2012-04-26 03:45:52]

LGTM.  Sorry, didn't mean to add myself as a reviewer.

[Sonny, 2012-04-26 04:55:18]

Some general comments about what data we're collecting and when.  Also wanted to
make sure we were going to grab the shmem and private memory info from chrome --
I think the call was GetProcessInfo?

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1818: p = subprocess.Popen('grep bytes
/sys/kernel/debug/dri/0/i915_gem_gtt',
GTT isn't guaranteed to always be there.  It'll only be present on systems using
Intel graphics, so you should test to see if it's there before opening that
file.  We also may just want to ignore GTT because it ends up being included in
Shmem -- see below comment.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1833: mem_dirty = re.search('Dirty:\s*([\d]+)
kB', stdout).group(1)
Dirty  Memory isn't really very interesting because it'll change quite a bit
based on when we write the pages out to disk.  More interesting would probaby be
AnonPages and Slab

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
I'm not sure what mem_available is supposed to mean here?

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1846: 
Also weren't we going to get the memory stats from Chrome using GetProcessInfo?

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1908: duration)
I'm not sure we want to record after opening every tab, probably just when the
whole set is opened?  I'm just worried it'll be noisy and too much information
to grok easily.

[ihf, 2012-04-26 05:19:16]

Sonny: could you explain somewhere what you want from GetProcessInfo? It is not
in the bug.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1818: p = subprocess.Popen('grep bytes
/sys/kernel/debug/dri/0/i915_gem_gtt',
Good catch on the checking for file existence.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
This was suggested by James as it is used for discarding tabs.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1846: 
First time I read about it.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1908: duration)
This works exactly as you want it.

[James Cook, 2012-04-26 05:25:58]

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
On 2012/04/26 05:19:17, ihf wrote:
> This was suggested by James as it is used for discarding tabs.

It's apparently what Luigi uses in the kernel to provide the low memory signal
to chrome.

[Sonny, 2012-04-26 05:38:13]

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1833: mem_dirty = re.search('Dirty:\s*([\d]+)
kB', stdout).group(1)
On 2012/04/26 04:55:18, Sonny wrote:
> Dirty  Memory isn't really very interesting because it'll change quite a bit
> based on when we write the pages out to disk.  More interesting would probaby
be
> AnonPages and Slab

Now that I understand what mem_available is supposed to be, i see why you're
reading these particular entries from meminfo.  But I still think we really want
a bunch more than just those numbers which are included in that calculation.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
On 2012/04/26 05:25:59, James Cook (Chromium) wrote:
> On 2012/04/26 05:19:17, ihf wrote:
> > This was suggested by James as it is used for discarding tabs.
> 
> It's apparently what Luigi uses in the kernel to provide the low memory signal
> to chrome.

Ah.. I see.  That's not actually exactly true either, because Luigi is also
using another number which is our floor on how much file cache we should allow
to stay.
If you really want that number, you need to read another proc file and factor it
in here.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1846: 
On 2012/04/26 05:19:17, ihf wrote:
> First time I read about it.
we want the private and shared and possibly the javascript memory in use for
each renderer.  Dennis knows what I'm talking about, as he has some code to go
do this.  Actually that reminds me, we also want a chrome flag which forces
every tab to get it's own renderer process.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1908: duration)
On 2012/04/26 05:19:17, ihf wrote:
> This works exactly as you want it.

Ahh oops, didn't see the indent, thanks.

[dennis_jeffrey, 2012-04-26 18:47:38]

Addressed comments and also added a new test that does the same thing, except it
opens up 10 live sites (as requested in the bug associated with this CL), and
uses a sampling period of 20 seconds instead of 15.

We do need to finalize the set of memory measurements we want to record, but
maybe we can get the current version checked in and running, then change or add
to the set of memory measurements in the next CL?

In case you're interested, here's the raw perf keyvals when I tried running the
new test (with live sites) on my local device:

_PERF_PRE_('KB_MemLive-MinGTT-0Tabs0', 50580.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxGTT-0Tabs0', 65704.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndGTT-0Tabs0', 50580.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemAvail-0Tabs0', 3228668.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemAvail-0Tabs0', 3260404.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemAvail-0Tabs0', 3260048.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemShare-0Tabs0', 86980.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemShare-0Tabs0', 102676.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemShare-0Tabs0', 86980.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinGTT-10Tabs1', 114964.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxGTT-10Tabs1', 119572.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndGTT-10Tabs1', 114964.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemAvail-10Tabs1', 2200048.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemAvail-10Tabs1', 2316096.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemAvail-10Tabs1', 2210924.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemShare-10Tabs1', 169136.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemShare-10Tabs1', 179408.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemShare-10Tabs1', 169404.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinGTT-0Tabs1', 66396.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxGTT-0Tabs1', 114996.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndGTT-0Tabs1', 66396.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemAvail-0Tabs1', 2944336.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemAvail-0Tabs1', 3160140.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemAvail-0Tabs1', 3158968.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemShare-0Tabs1', 93784.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemShare-0Tabs1', 157084.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemShare-0Tabs1', 93784.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinGTT-10Tabs2', 122976.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxGTT-10Tabs2', 127848.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndGTT-10Tabs2', 122976.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemAvail-10Tabs2', 2176064.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemAvail-10Tabs2', 2210308.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemAvail-10Tabs2', 2203292.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemShare-10Tabs2', 174664.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemShare-10Tabs2', 179624.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemShare-10Tabs2', 175012.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinGTT-0Tabs2', 75804.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxGTT-0Tabs2', 123072.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndGTT-0Tabs2', 75804.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemAvail-0Tabs2', 2903888.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemAvail-0Tabs2', 3116224.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemAvail-0Tabs2', 3116064.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MinMemShare-0Tabs2', 103220.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-MaxMemShare-0Tabs2', 161544.000000)_PERF_POST_

_PERF_PRE_('KB_MemLive-EndMemShare-0Tabs2', 103220.000000)_PERF_POST_

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1818: p = subprocess.Popen('grep bytes
/sys/kernel/debug/dri/0/i915_gem_gtt',
On 2012/04/26 05:19:17, ihf wrote:
> Good catch on the checking for file existence.

I modified the code to exclude GTT values if the file doesn't exist.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1833: mem_dirty = re.search('Dirty:\s*([\d]+)
kB', stdout).group(1)
On 2012/04/26 05:38:14, Sonny wrote:
> On 2012/04/26 04:55:18, Sonny wrote:
> > Dirty  Memory isn't really very interesting because it'll change quite a bit
> > based on when we write the pages out to disk.  More interesting would
probaby
> be
> > AnonPages and Slab
> 
> Now that I understand what mem_available is supposed to be, i see why you're
> reading these particular entries from meminfo.  But I still think we really
want
> a bunch more than just those numbers which are included in that calculation.

See my comment at line 1846 below.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
On 2012/04/26 05:38:14, Sonny wrote:
> On 2012/04/26 05:25:59, James Cook (Chromium) wrote:
> > On 2012/04/26 05:19:17, ihf wrote:
> > > This was suggested by James as it is used for discarding tabs.
> > 
> > It's apparently what Luigi uses in the kernel to provide the low memory
signal
> > to chrome.
> 
> Ah.. I see.  That's not actually exactly true either, because Luigi is also
> using another number which is our floor on how much file cache we should allow
> to stay.
> If you really want that number, you need to read another proc file and factor
it
> in here.

Let me know how the computation for available memory needs to change here.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1846: 
On 2012/04/26 05:38:14, Sonny wrote:
> On 2012/04/26 05:19:17, ihf wrote:
> > First time I read about it.
> we want the private and shared and possibly the javascript memory in use for
> each renderer.  Dennis knows what I'm talking about, as he has some code to go
> do this.  Actually that reminds me, we also want a chrome flag which forces
> every tab to get it's own renderer process.

Yes, there is lots more information that people want to collect.  There's the
information Sonny mentioned (which as he said, we've already implemented before
when conducting other experiments).  Additionally, Puneet mentioned that he'd
like to see some other info too (need to check in with him again to get the
details).

I recommend getting a first version of this test checked in and running, then we
can add or modify the set of measurements taken in another CL, once we've hashed
out what we need.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1908: duration)
On 2012/04/26 05:38:14, Sonny wrote:
> On 2012/04/26 05:19:17, ihf wrote:
> > This works exactly as you want it.
> 
> Ahh oops, didn't see the indent, thanks.

Done - working as intended.

[ilja, 2012-04-26 21:37:12]

I agree: lets iterate once we get a few numbers. The changes look good.

[Sonny, 2012-04-26 22:25:40]

I guess I'm in the minority here, but in it's current form I think this test
will only tell us what memory is being used for graphics.  If we're going to
quickly add more stuff, I don't see why we shouldn't just do it now.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
On 2012/04/26 18:47:39, dennis_jeffrey wrote:
> On 2012/04/26 05:38:14, Sonny wrote:
> > On 2012/04/26 05:25:59, James Cook (Chromium) wrote:
> > > On 2012/04/26 05:19:17, ihf wrote:
> > > > This was suggested by James as it is used for discarding tabs.
> > > 
> > > It's apparently what Luigi uses in the kernel to provide the low memory
> signal
> > > to chrome.
> > 
> > Ah.. I see.  That's not actually exactly true either, because Luigi is also
> > using another number which is our floor on how much file cache we should
allow
> > to stay.
> > If you really want that number, you need to read another proc file and
factor
> it
> > in here.
> 
> Let me know how the computation for available memory needs to change here.

so it should be
MemFree + Inactive file + active file - dirty - min_file_mem

and min_file_mem comes from /proc/sys/vm/min_filelist_kbytes

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1846: 
On 2012/04/26 18:47:39, dennis_jeffrey wrote:
> On 2012/04/26 05:38:14, Sonny wrote:
> > On 2012/04/26 05:19:17, ihf wrote:
> > > First time I read about it.
> > we want the private and shared and possibly the javascript memory in use for
> > each renderer.  Dennis knows what I'm talking about, as he has some code to
go
> > do this.  Actually that reminds me, we also want a chrome flag which forces
> > every tab to get it's own renderer process.
> 
> Yes, there is lots more information that people want to collect.  There's the
> information Sonny mentioned (which as he said, we've already implemented
before
> when conducting other experiments).  Additionally, Puneet mentioned that he'd
> like to see some other info too (need to check in with him again to get the
> details).
> 
> I recommend getting a first version of this test checked in and running, then
we
> can add or modify the set of measurements taken in another CL, once we've
hashed
> out what we need.

Ok... I just feel like we'll be adding more stuff for collection soon after this
lands, because what we are collecting now doesn't help us figure out what is
consuming the memory beyond GTT and Shmem.

[dennis_jeffrey, 2012-04-28 00:21:35]

I changed GTT memory to the Gem Objects memory.  I'm also recording several new
types of memory values requested by Sonny.  For renderer processes, I'm just
computing the total sum right now.  If we want to view it on a per-tab basis, I
can add that in the next CL.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1840: int(mem_free))
On 2012/04/26 22:25:40, Sonny wrote:
> On 2012/04/26 18:47:39, dennis_jeffrey wrote:
> > On 2012/04/26 05:38:14, Sonny wrote:
> > > On 2012/04/26 05:25:59, James Cook (Chromium) wrote:
> > > > On 2012/04/26 05:19:17, ihf wrote:
> > > > > This was suggested by James as it is used for discarding tabs.
> > > > 
> > > > It's apparently what Luigi uses in the kernel to provide the low memory
> > signal
> > > > to chrome.
> > > 
> > > Ah.. I see.  That's not actually exactly true either, because Luigi is
also
> > > using another number which is our floor on how much file cache we should
> allow
> > > to stay.
> > > If you really want that number, you need to read another proc file and
> factor
> > it
> > > in here.
> > 
> > Let me know how the computation for available memory needs to change here.
> 
> so it should be
> MemFree + Inactive file + active file - dirty - min_file_mem
> 
> and min_file_mem comes from /proc/sys/vm/min_filelist_kbytes

Done.

https://chromiumcodereview.appspot.com/10161033/diff/10002/chrome/test/functi...
chrome/test/functional/perf.py:1846: 
On 2012/04/26 22:25:40, Sonny wrote:
> On 2012/04/26 18:47:39, dennis_jeffrey wrote:
> > On 2012/04/26 05:38:14, Sonny wrote:
> > > On 2012/04/26 05:19:17, ihf wrote:
> > > > First time I read about it.
> > > we want the private and shared and possibly the javascript memory in use
for
> > > each renderer.  Dennis knows what I'm talking about, as he has some code
to
> go
> > > do this.  Actually that reminds me, we also want a chrome flag which
forces
> > > every tab to get it's own renderer process.
> > 
> > Yes, there is lots more information that people want to collect.  There's
the
> > information Sonny mentioned (which as he said, we've already implemented
> before
> > when conducting other experiments).  Additionally, Puneet mentioned that
he'd
> > like to see some other info too (need to check in with him again to get the
> > details).
> > 
> > I recommend getting a first version of this test checked in and running,
then
> we
> > can add or modify the set of measurements taken in another CL, once we've
> hashed
> > out what we need.
> 
> Ok... I just feel like we'll be adding more stuff for collection soon after
this
> lands, because what we are collecting now doesn't help us figure out what is
> consuming the memory beyond GTT and Shmem.

We're now recording more different types of memory.  We can decide later which
subset of this to actually plot on graphs.

[Sonny, 2012-04-28 00:40:01]

LGTM Looks good thanks! Had one more suggestion, but not required for the first
pass

https://chromiumcodereview.appspot.com/10161033/diff/18001/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/18001/chrome/test/functi...
chrome/test/functional/perf.py:1770: 
Forgot to put this in it's own comment last time, should we add the
--process-per-tab flag using ExtraFlags, in order to make sure we get consistent
results?

[dennis_jeffrey, 2012-04-28 00:55:17]

https://chromiumcodereview.appspot.com/10161033/diff/18001/chrome/test/functi...
File chrome/test/functional/perf.py (right):

https://chromiumcodereview.appspot.com/10161033/diff/18001/chrome/test/functi...
chrome/test/functional/perf.py:1770: 
On 2012/04/28 00:40:02, Sonny wrote:
> Forgot to put this in it's own comment last time, should we add the
> --process-per-tab flag using ExtraFlags, in order to make sure we get
consistent
> results?

Done.

[Sonny, 2012-04-28 00:56:43]

lgtm

[ilja, 2012-04-28 01:01:42]

lgtm

[James Cook, 2012-04-28 23:03:56]

lgtm