1457243 - Measure best case process memory usage on AWSY

Status: RESOLVED FIXED

(Testing :: AWSY, enhancement)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

The goal is to measure and graph the best case content process size. This allows us to track the memory feasibility of a large number of content processes.

It should ignore shared memory components so could perhaps be measured as the difference between n and n+1 tabs of about:blank being loaded in the browser.

Comment 1

[Eric Rahm [:erahm]]

Just a note: this number is going to be pretty noisy. We might try something like loading 4 about:blanks, waiting 30-60s, performing a GC and then only grabbing the measurement of the 'last' about:blank process. This could just end up being the 'SY-e10s(ab)' measurement.

We can probably just use the existing framework, but one thing we want to watch out for is any marionette overhead in the content process.

Comment 2

[Eric Rahm [:erahm]]

I have WIP cloned off the existing awsy test, I just need to clean it up and decide on how much code we can share.

Comment 3

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fdcdfe654f31c742613acc667eb7d1361ae94a34

Comment 4

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3403506ecf14cd39aae46cd0791638ee9572e683

Comment 5

[Bob Clary [:bc] (inactive)]

Attachment: test-sets.patch

try this.

Comment 6

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae7ff64691b50f9b81c42a94c722a73c6fba0ef0

Comment 7

[Eric Rahm [:erahm]]

Attachment: Part 1: Support zero values for awsy params in mach_commands

Comment 8

[Eric Rahm [:erahm]]

Attachment: Part 2: Allow checkpoints and perf_suites to be passed in to process_perf_data

Comment 9

[Eric Rahm [:erahm]]

Attachment: Part 3: Split out a base awsy test case

Comment 10

[Eric Rahm [:erahm]]

Attachment: Part 4: Support filtering on process names and limit process count

This adds support for only gathering data for a given process type. It also
allows you to limit the number of processes included in a measurement.

Comment 11

[Eric Rahm [:erahm]]

Attachment: Part 5: Add base case AWSY test that just measures about:blank

Comment 12

[Eric Rahm [:erahm]]

Attachment: Part 6: Hook in the about:blank awsy test to automation

Comment 13

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a6f65bc7e7da1d59871901f4b0ebaea1fad161b

Comment 14

[Bob Clary [:bc] (inactive)]

I'm out today but I'll get to this this afternoon/evening when I get back.

Comment 15

[Bob Clary [:bc] (inactive)]

Comment on attachment 8974580 [details] [diff] [review]
Part 1: Support zero values for awsy params in mach_commands

Review of attachment 8974580 [details] [diff] [review]:
-----------------------------------------------------------------

Looks ok but if you can simplify it that would be nice. r+

::: testing/awsy/mach_commands.py
@@ +81,5 @@
> +        # These args can be zero
> +        for arg in ('entities', 'iterations', 'perTabPause', 'settleWaitTime', 'maxTabs'):
> +            if arg in kwargs and kwargs[arg] is not None:
> +                runtime_testvars[arg] = kwargs[arg]
> +

Perhaps I'm missing it, but couldn't we do this for all of the args instead of having both?

Comment 16

[Bob Clary [:bc] (inactive)]

Comment on attachment 8974581 [details] [diff] [review]
Part 2: Allow checkpoints and perf_suites to be passed in to process_perf_data

Review of attachment 8974581 [details] [diff] [review]:
-----------------------------------------------------------------

r+, looks ok.

::: testing/awsy/awsy/process_perf_data.py
@@ +36,3 @@
>      """
>      Updates CHECKPOINTS with memory report file fetched in data_path
>      :param checkpoint_files: list of files in data_path

Document checkpoints?

Comment 17

[Bob Clary [:bc] (inactive)]

Comment on attachment 8974582 [details] [diff] [review]
Part 3: Split out a base awsy test case

Review of attachment 8974582 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with fixes for the imports.

::: testing/awsy/awsy/test_memory_usage.py
@@ +5,2 @@
>  import os
>  import sys

from marionette_driver.errors import JavascriptException, ScriptTimeoutException

are still required in clear_preloaded_browser()

@@ -26,1 @@
>  from awsy import process_perf_data, webservers

webservers no longer used.

Comment 18

[Bob Clary [:bc] (inactive)]

Comment on attachment 8974583 [details] [diff] [review]
Part 4: Support filtering on process names and limit process count

Review of attachment 8974583 [details] [diff] [review]:
-----------------------------------------------------------------

r+

Comment 19

[Bob Clary [:bc] (inactive)]

Comment on attachment 8974584 [details] [diff] [review]
Part 5: Add base case AWSY test that just measures about:blank

Review of attachment 8974584 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with comment addressed.

::: testing/awsy/awsy/test_base_memory_usage.py
@@ +42,5 @@
> +
> +    def perf_checkpoints(self):
> +        return CHECKPOINTS
> +
> +    def iterations(self):

I'm a little concerned about overriding this to return a fixed number but not also dealing with the self._iterations.

Wouldn't it be better to just override self._iterations = 1 in setUp and leave this alone?

Comment 20

[Eric Rahm [:erahm]]

(In reply to Bob Clary [:bc:] from comment #15)
> Comment on attachment 8974580 [details] [diff] [review]
> Part 1: Support zero values for awsy params in mach_commands
> 
> Review of attachment 8974580 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks ok but if you can simplify it that would be nice. r+
> 
> ::: testing/awsy/mach_commands.py
> @@ +81,5 @@
> > +        # These args can be zero
> > +        for arg in ('entities', 'iterations', 'perTabPause', 'settleWaitTime', 'maxTabs'):
> > +            if arg in kwargs and kwargs[arg] is not None:
> > +                runtime_testvars[arg] = kwargs[arg]
> > +
> 
> Perhaps I'm missing it, but couldn't we do this for all of the args instead
> of having both?

It's not quite the same test, like if kwargs['webroot'] were an empty string that would evaluate to False as well, but I guess that's okay. If someone specifies an empty string they probably mean it.

Comment 21

[Eric Rahm [:erahm]]

(In reply to Bob Clary [:bc:] from comment #16)
> Comment on attachment 8974581 [details] [diff] [review]
> Part 2: Allow checkpoints and perf_suites to be passed in to
> process_perf_data
> 
> Review of attachment 8974581 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+, looks ok.
> 
> ::: testing/awsy/awsy/process_perf_data.py
> @@ +36,3 @@
> >      """
> >      Updates CHECKPOINTS with memory report file fetched in data_path
> >      :param checkpoint_files: list of files in data_path
> 
> Document checkpoints?

Updated.

Comment 22

[Eric Rahm [:erahm]]

(In reply to Bob Clary [:bc:] from comment #17)
> Comment on attachment 8974582 [details] [diff] [review]
> Part 3: Split out a base awsy test case
> 
> Review of attachment 8974582 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with fixes for the imports.
> 
> ::: testing/awsy/awsy/test_memory_usage.py
> @@ +5,2 @@
> >  import os
> >  import sys
> 
> from marionette_driver.errors import JavascriptException,
> ScriptTimeoutException
> 
> are still required in clear_preloaded_browser()

Oh right, I got a bit overzealous.

> @@ -26,1 @@
> >  from awsy import process_perf_data, webservers
> 
> webservers no longer used.

It's still used in test_memory_usage, but I'll remove it from awsy_test_case.

Comment 23

[Eric Rahm [:erahm]]

(In reply to Bob Clary [:bc:] from comment #19)
> Comment on attachment 8974584 [details] [diff] [review]
> Part 5: Add base case AWSY test that just measures about:blank
> 
> Review of attachment 8974584 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with comment addressed.
> 
> ::: testing/awsy/awsy/test_base_memory_usage.py
> @@ +42,5 @@
> > +
> > +    def perf_checkpoints(self):
> > +        return CHECKPOINTS
> > +
> > +    def iterations(self):
> 
> I'm a little concerned about overriding this to return a fixed number but
> not also dealing with the self._iterations.
> 
> Wouldn't it be better to just override self._iterations = 1 in setUp and
> leave this alone?

Sounds reasonable, I'll update.

Comment 24

[Eric Rahm [:erahm]]

Thanks for the reviews Bob!

Comment 25

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c216f3f88f9b4e94346db7b1a4ad9c9b300370a7
Bug 1457243 - Part 1: Support zero values for awsy params in mach_commands. r=bc

https://hg.mozilla.org/integration/mozilla-inbound/rev/0629e4a05e5cf1f665957d909bcbcb0b98877d88
Bug 1457243 - Part 2: Allow checkpoints and perf_suites to be passed in to process_perf_data. r=bc

https://hg.mozilla.org/integration/mozilla-inbound/rev/9091f8ac9bbd4b9d9503d57f0cd149293fd0f0c2
Bug 1457243 - Part 3: Split out a base awsy test case. r=bc

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b2f923280eb57dd4d6e51163b362cb7db728620
Bug 1457243 - Part 4: Support filtering on process names and limit process count. r=bc

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbe6ab7db4f7aa15502f37004f0f94c724d6d6fe
Bug 1457243 - Part 5: Add base case AWSY test that just measures about:blank. r=bc

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbf88f9bacbb5568ca28d94deaa604856a2dc336
Bug 1457243 - Part 6: Hook in the about:blank awsy test to automation. r=bc

Comment 26

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8af38d67ffb3efb94f619b1f5b00ccaa19609263
Bug 1457243 - Part 7: Followup - Setup base case testvars. r=me

Comment 27

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/91f9e23430b3f41bcc6cd3647864d9856930561a
Bug 1457243 - Fix flake8. r=me, test-only DONTBUILD CLOSED TREE

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c216f3f88f9b
https://hg.mozilla.org/mozilla-central/rev/0629e4a05e5c
https://hg.mozilla.org/mozilla-central/rev/9091f8ac9bbd
https://hg.mozilla.org/mozilla-central/rev/9b2f923280eb
https://hg.mozilla.org/mozilla-central/rev/cbe6ab7db4f7
https://hg.mozilla.org/mozilla-central/rev/fbf88f9bacbb
https://hg.mozilla.org/mozilla-central/rev/8af38d67ffb3
https://hg.mozilla.org/mozilla-central/rev/91f9e23430b3