1383146 - add new talos test to q1s that measures STYLO_THREADS=1

Status: RESOLVED FIXED

(Testing :: Talos, enhancement)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

currently we run quantum pageload tests on regular firefox.

As of bug 1382524, we started running with stylo turned on.

We want a 3rd data point to expand on just turning stylo on and adding the environment variable STYLO_THREADS=1.


We could save some headaches and avoid the buildbot changes by running this in the same q1s job.

We would need to:
* modify talos to define the new test quantum-pageload-stylo-threads
* add a cli arg or a hack to set the STYLO_THREADS=1 environment variable
* modify testing/talos/talos.json to run this test as well
* modify taskcluster/ci/tests.yml to have a definition for this new test (I think just a reference to it is needed)
* ensure we report something useful into perfherder

Right now perfherder sees "quantum pageload opt e10s stylo", adding "threads" to the end will get long, is there a better way to describe the options to perfherder?

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:rwood, any further thoughts here, things I might have missed, etc.

Comment 2

[Robert Wood [:rwood]]

The configs are a huge pain but I think it might be best to keep it separate, and have a new symbol 'q1st'. Just makes it more flexible in case we want to easily turn it off, only run it on specific platforms in the future, re-triggering only that test when investigating regressions, have it included in SETA, etc.

Comment 3

[Robert Wood [:rwood]]

See Bug 1383737 regarding test &  job names

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot-config changes for tp6

this patch will:
1) add tp6-stylo-threads-e10s to be scheduled by default (note the new name)
2) add tp6-e10s and tp6-stylo-e10s as options for try (until we get that right and switch quantum-pageload off)

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: buildbot_configs diff: tp6_diff.txt

these are the new jobs!

Comment 6

[Robert Wood [:rwood]]

Comment on attachment 8890117 [details] [diff] [review]
buildbot-config changes for tp6

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

::: mozilla-tests/config.py
@@ +334,5 @@
> +        'options': ({}, WIN_ONLY),
> +    },
> +    'tp6-stylo-threads-e10s': {
> +        'enable_by_default': True,
> +        'suites': GRAPH_CONFIG + ['--activeTests', 'Quantum_1', '--filter', 'ignore_first:5', '--filter', 'median', '--stylo-threads'],

I just realized, how is this even working with 'Quantum_1'? That test doesn't even exist, now we use

"tests": ["quantum_pageload_google", "quantum_pageload_youtube", "quantum_pageload_amazon", "quantum_pageload_facebook"],

Comment 7

[Joel Maher ( :jmaher ) (UTC -8)]

inside of buildbot these are just placeholders, the real definition is in-tree in the testing/talos/talos.json file.

Comment 8

[Robert Wood [:rwood]]

Comment on attachment 8890117 [details] [diff] [review]
buildbot-config changes for tp6

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

Ok, LGTM

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: support --stylo-threads in talos

this does a few things:
* add tp6 names for tests (the future direction)
* adds the ability for specifying a specific amount of threads via the cli variable --stylo-threads=X
* will report as tp6_<page> X_threads e10s stylo

I left this as a secondary option to require --stylo as well- not sure if we should change that, I view this as a temporary (<6 month) test.

here is a try push and I hacked up q1s to have threads:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e44f8490f11f680f02c7614e72e2a3488fa6dfee

Comment 10

[Robert Wood [:rwood]]

Comment on attachment 8890475 [details] [diff] [review]
support --stylo-threads in talos

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

::: testing/talos/talos/cmdline.py
@@ +173,5 @@
>              help='If given, enable Stylo via Environment variables and '
>                   'upload results with Stylo options.')
> +    add_arg('--stylo-threads', type=int,
> +            dest='stylothreads',
> +            help='If given, Run stylo with a certain number of threads')

nit: don't need capital "Run"

::: testing/talos/talos/run_tests.py
@@ +192,5 @@
>      if config['stylo']:
>          talos_results.add_extra_option('stylo')
>  
> +    # measuring the difference of a a certain thread level
> +    if 'stylothreads' in config and config['stylothreads'] > 0:

nit, bit shorter:

if config.get('stylothreads', 0) > 0

::: testing/talos/talos/ttest.py
@@ +97,5 @@
>              setup.env['STYLO_FORCE_ENABLED'] = 1
>  
> +        # During the Stylo transition, measure different number of threads
> +        if browser_config['stylothreads']:
> +            setup.env['STYLO_THREADS'] = 1

should be set to the 'stylothreads' value, not hardcoded to 1

Comment 11

[Robert Wood [:rwood]]

Comment on attachment 8890475 [details] [diff] [review]
support --stylo-threads in talos

Looks awesome, one issue and couple of nits (see Comment 10)

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

Attachment: support --stylo-threads in talos

thanks for the issues yesterday!

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29efb0ac89cf
add new talos test to q1s that measures STYLO_THREADS=1. r=rwood

Comment 14

[Nick Thomas [:nthomas] (UTC+12)]

Merged to production - https://hg.mozilla.org/build/buildbot-configs/rev/534f8494d95bf8e7730f75c44c9cf62439474d51

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/29efb0ac89cf

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

all the plumbing is in place, we just need a small patch to turn this on.