Take 'ancestor-manifest' into account in the 'chunk_by_runtime' algorithm
Categories
(Testing :: Mozbase, task, P1)
Tracking
(firefox73 fixed)
Tracking | Status | |
---|---|---|
firefox73 | --- | fixed |
People
(Reporter: ahal, Assigned: ahal)
References
Details
Attachments
(3 files)
Some manifests are included by others. These are not registered directly in moz.build
files, rather they get included by other manifests. These parents can specify defaults that propagate down to the child, so we need to make sure we never split these child manifests from their parents.
Assignee | ||
Comment 1•4 years ago
|
||
Also rename the key from 'ancestor-manifest' to 'ancestor_manifest' to be
consistent with other keys.
Assignee | ||
Comment 2•4 years ago
|
||
Ensures child (aka included) manifests always run in the same chunk as their parent.
Depends on D55283
Assignee | ||
Comment 3•4 years ago
|
||
Some manifests get included by parent manifests (e.g [include:<child
manifest>]). The runtime of these manifests should be rolled into the runtime
of the parent manifest as that is how it will be run in CI.
This change also updates the runtimes files to incorporate this change.
Depends on D55284
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d28d71c964c7 [manifestparser] Make 'ancestor_manifest' a relative path, r=gbrown https://hg.mozilla.org/integration/autoland/rev/3c4063b93ce7 [manifestparser] Use 'ancestor_manifest' in the 'chunk_by_runtime' filter (if it exists), r=gbrown https://hg.mozilla.org/integration/autoland/rev/4c4e74df38bd [runtimes] Don't store runtime information for included manifests, r=gbrown
Comment 5•4 years ago
|
||
Backed out 3 changesets for causing bc and dt failures regarding manifest filters.
Backout link: https://hg.mozilla.org/integration/autoland/rev/b9ef4475bda2b491ef377cd16e923e5e92c5de35
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279063885&repo=autoland&lineNumber=1447
Comment 6•4 years ago
|
||
It also seems to be causing mda failures on android: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=598800&searchStr=android%2C8.0%2Cpixel2%2Cdebug%2Cmochitests%2Ctest-android-hw-p2-8-0-arm7-api-16%2Fdebug-geckoview-mochitest-media-e10s-2%2Cm%28mda2%29&tochange=71af6290f43656a1b3d978428d920e3a7dc35b12&fromchange=9dea219937cd181806ec08f5f6e55a6d4685fef4&selectedJob=279067598
Assignee | ||
Comment 7•4 years ago
|
||
This happened because in CI we generate a "root" manifest in the test package that includes every other manifest. So this generated "root" manifest was listed as the "ancestor_manifest" of every single test, resulting in only a single manifest to chunk across all chunks.
While I tested this change, it was alongside the main taskgraph chunking series, which moves the chunking logic to the decision task (where we don't deal with test packages).
One potential strategy is to roll the chunk_by_runtime
change into the larger series so we don't get into this intermediate state. Though doing this would mean we need to support at least browser-chrome, devtools and mochitest-media right out of the gate.
Assignee | ||
Comment 8•4 years ago
|
||
I fixed it by ignoring "root" manifests in the ancestor calculation:
https://treeherder.mozilla.org/#/jobs?repo=try&author=ahalberstadt%40mozilla.com&fromchange=7b255d909789877e6b27c27b4ee6f6da4e79bcb5&tochange=e0bf347c881930932849ec00d962de78a7acda3b
Pushed by ahalberstadt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a147516fef23 [manifestparser] Make 'ancestor_manifest' a relative path, r=gbrown https://hg.mozilla.org/integration/autoland/rev/ceffb2fcecdd [runtimes] Don't store runtime information for included manifests, r=gbrown https://hg.mozilla.org/integration/autoland/rev/07a2031c69c8 [manifestparser] Use 'ancestor_manifest' in the 'chunk_by_runtime' filter (if it exists), r=gbrown
Comment 10•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a147516fef23
https://hg.mozilla.org/mozilla-central/rev/ceffb2fcecdd
https://hg.mozilla.org/mozilla-central/rev/07a2031c69c8
Description
•