Closed Bug 1803361 Opened 2 years ago Closed 2 years ago

Handle query negation removals outside of the variant expansion

Categories

(Testing :: Performance, defect, P1)

defect

Tracking

(firefox109 fixed)

RESOLVED FIXED
109 Branch
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.

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.

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.

Assignee: nobody → gmierz2
Status: NEW → ASSIGNED
Attachment #9306108 - Attachment is obsolete: true

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.

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

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

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

Blocks: 1804436
Blocks: 1804442
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: