Handle query negation removals outside of the variant expansion
Categories
(Testing :: Performance, defect, P1)
Tracking
(firefox109 fixed)
| Tracking | Status | |
|---|---|---|
| firefox109 | --- | fixed |
People
(Reporter: sparky, Assigned: sparky)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
The negation query removal code is currently only run when we are expanding variants, but we should also check for this in non-variant tasks.
|
Assignee | |
Comment 1•1 year ago
|
||
Instead of fixing this issue, I'm going to remove the live-site category. It really doesn't make sense to have it when we have live-sites as a variant, and this can be applied to our existing pageload categories to achieve the same thing.
Also, I'm going to add a restriction to the category specification (as a comment), to make sure we don't introduce variants into the categories. They should always be distinct groups.
|
|
Assignee | |
Comment 2•1 year ago
|
||
There was a bug found regarding how this category was empty because the negation of live sites !live was inadvertently included. This is expected given the code we have. Instead of changing the behaviour of mach try perf when it comes to handling negation removals in the variants, we'll remove the live-site pageload category and add a restriction to the category definitions. The tests are also modified to add new tests for this restriction, and change up the tests to ensure they aren't using bad categories.
|
||
Updated•1 year ago
|
|
|
||
Updated•1 year ago
|
|
|
Assignee | |
Comment 3•1 year ago
|
||
There was a bug found regarding how the Pageload (live) category was empty because the negation of live sites !live was inadvertently included. This is expected given the code we have. Instead of changing the behaviour of mach try perf when it comes to handling negation removals in the variants, we'll remove the live-site pageload category and add a restriction to the category definitions. At the same time, the Bytecode-Cached category is removed since it's a variant of some existing tests as well. Note that these removals are done in a later patch with a few other changes.
The tests are also modified to add new tests for this restriction, and change up the tests to ensure they aren't using bad categories.
|
|
Assignee | |
Comment 4•1 year ago
|
||
This patch adds the base method that is used to build our decision matrix for use in building up our categories. The decision matrix has the form: [
[
[
# Android platform, with apps as columns (firefox, chrome, chromium, fenix, geckoview, chrome-m)
[False, False, False, True, True, True]
# Android-a51
[False, ...]
[...]
[...]
[...]
[...]
] # Raptor
[
...
] # Talos
[] # AWSY
], # Base variant (uncombined, e.g. only live-sites or profiling)
[], # Another variant
]
Each element in this matrix tells us whether this particular (variant, suite, platform, app) combination is valid and runnable. If the element is False then the combination is not runnable. We make use of this to build up our categories and variant combinations. In the next patch, the variant-combination decision matrices are made by performing an AND operation on all of the corresponding elements in the individual variant decision matrices.
Depends on D163836
|
|
Assignee | |
Comment 5•1 year ago
|
||
This patch adds a method that builds decision matrices (see previous patch for info on this) for the category, and all variant combinations. At this stage, user requests (platforms, apps, and variants) get applied along with category restrictions. Furthermore, the restricted apps, and platforms are handled here. The next patch in this series shows how the categories are setup for this.
The category matrix has the form: {
category-name: {
variant_combination_1: variants_decision_matrix_1,
variant_combination_2: variants_decision_matrix_2,
...
}
}
Note how the category name is the base category name and hasn't been combined into a formal name for the selector to display. This is expected as the name creation happens within the expand_categories method.
Depends on D163837
|
|
Assignee | |
Comment 6•1 year ago
|
||
This patch series replaces the existing expand_categories method with a new one that makes us of the decision matrices to build the categories to display. The category matrix contains all combinations of variants, suites, platforms, and apps, for this particular category and each element represents whether some combination (variant, suite, platform, app) can run in this category. True means it can run, False means it can't.
At the end of the category expansions phase, we go through the categories to display and apply the restriction/negation queries to them. Doing this at the end allows us to be more precise in where we apply these. Note that the tests category, and variants were updated because this was a major change. Given the new restrictions on the categories, the number of categories are dramatically reduced. This is because we strengthened the criteria for being selected from only one variant being available, to all variants being available to run.
This new method with matrices uses more code, and memory versus the previous method, but it's much more maintainable and has new features to prevent the modification of the core code of the selector. In the previous solution, we needed to modify the core code when we added new restrictions. Now, this is all defined outside of the PerfParser. It's also much easier to tell where things are happening. Before, the code was very concentrated and a small change often caused unwanted/unexpected changes.
Depends on D163838
Pushed by gmierz2@outlook.com: https://hg.mozilla.org/integration/autoland/rev/f9637dac96c9 Add pre-run category checks to perf-selector. r=perftest-reviewers,kshampur https://hg.mozilla.org/integration/autoland/rev/495d2201467b Add a method to build a decision matrix. r=perftest-reviewers,kshampur https://hg.mozilla.org/integration/autoland/rev/d86acb4b212e Build decision matrices for all categories. r=perftest-reviewers,kshampur https://hg.mozilla.org/integration/autoland/rev/bc5138fb7171 Use decision matrices for the perf-selector category expansion. r=perftest-reviewers,kshampur
|
||
Comment 8•1 year ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/f9637dac96c9
https://hg.mozilla.org/mozilla-central/rev/495d2201467b
https://hg.mozilla.org/mozilla-central/rev/d86acb4b212e
https://hg.mozilla.org/mozilla-central/rev/bc5138fb7171
Description
•