Closed
Bug 1383146
Opened 7 years ago
Closed 7 years ago
add new talos test to q1s that measures STYLO_THREADS=1
Categories
(Testing :: Talos, enhancement)
Testing
Talos
Tracking
(firefox56 fixed)
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: jmaher, Assigned: jmaher)
References
(Blocks 1 open bug)
Details
(Whiteboard: [PI:July] [stylo])
Attachments
(3 files, 1 obsolete file)
2.64 KB,
patch
|
rwood
:
review+
|
Details | Diff | Splinter Review |
2.23 KB,
text/plain
|
Details | |
6.29 KB,
patch
|
rwood
:
review+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 1•7 years ago
|
||
:rwood, any further thoughts here, things I might have missed, etc.
Flags: needinfo?(rwood)
Blocks: stylo-tooling
Comment 2•7 years ago
|
||
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.
Flags: needinfo?(rwood)
Updated•7 years ago
|
Whiteboard: [PI:July] → [PI:July] [stylo]
Assignee | ||
Comment 4•7 years ago
|
||
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)
Assignee: nobody → jmaher
Attachment #8890117 -
Flags: review?(rwood)
Assignee | ||
Comment 5•7 years ago
|
||
these are the new jobs!
Comment 6•7 years ago
|
||
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"],
Assignee | ||
Comment 7•7 years ago
|
||
inside of buildbot these are just placeholders, the real definition is in-tree in the testing/talos/talos.json file.
Comment 8•7 years ago
|
||
Comment on attachment 8890117 [details] [diff] [review] buildbot-config changes for tp6 Review of attachment 8890117 [details] [diff] [review]: ----------------------------------------------------------------- Ok, LGTM
Attachment #8890117 -
Flags: review?(rwood) → review+
Assignee | ||
Comment 9•7 years ago
|
||
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
Attachment #8890475 -
Flags: review?(rwood)
Comment 10•7 years ago
|
||
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•7 years ago
|
||
Comment on attachment 8890475 [details] [diff] [review] support --stylo-threads in talos Looks awesome, one issue and couple of nits (see Comment 10)
Attachment #8890475 -
Flags: review?(rwood) → review-
Assignee | ||
Comment 12•7 years ago
|
||
thanks for the issues yesterday!
Attachment #8890475 -
Attachment is obsolete: true
Attachment #8890871 -
Flags: review?(rwood)
Updated•7 years ago
|
Attachment #8890871 -
Flags: review?(rwood) → review+
Comment 13•7 years ago
|
||
Pushed by jmaher@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/29efb0ac89cf add new talos test to q1s that measures STYLO_THREADS=1. r=rwood
Comment 14•7 years ago
|
||
Merged to production - https://hg.mozilla.org/build/buildbot-configs/rev/534f8494d95bf8e7730f75c44c9cf62439474d51
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/29efb0ac89cf
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Assignee | ||
Comment 16•7 years ago
|
||
all the plumbing is in place, we just need a small patch to turn this on.
You need to log in
before you can comment on or make changes to this bug.
Description
•