Remove the separate vismet task, and run it within the test tasks
Categories
(Testing :: Raptor, task, P1)
Tracking
(Not tracked)
People
(Reporter: sparky, Assigned: sparky)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [fxp])
Attachments
(7 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
We should get rid of the separate vismet task and run them in the test task instead. With this change, it will make it simpler to standardize how we run visual metrics in-tree and it would also solve the retrigger issue.
There aren't many benefits from running vismet separately other than saving a few minutes of CI time. But this comes with the con of having developers wait longer for their tests to finish to get vismet numbers.
Assignee | ||
Comment 1•3 years ago
|
||
An idea is to use an AWS service to perform the calculations
Assignee | ||
Comment 2•3 years ago
|
||
I've started looking into this issue as I consider it a blocker for improving the try-testing experience.
I don't think that having a microservice is a good idea because it would virtually be the same thing as what we have plus the additional burden of maintaining another system outside of mozilla-central and some large additional costs.
I don't think we can or should integrate vismet into the same task now for these reasons:
- Large amount of effort for a medium-small impact
- Waste of time if we rewrite the visual metrics script to be more portable
- Maintenance will be burdensome because of the large number of platforms we target
- Test tasks will fail intermittently more often -> More data loss
To me, we have two solutions that we should pursue here:
-
LONG-TERM SOLUTION: We should look into replacing/updating the script to be more portable otherwise we'll continuously lose time to maintaining this relatively old script.
-
QUICK SOLUTION: Make a new dependency that shows the results and depends on the test task and the vismet task, and hide other tasks in the try screens. Push people to try chooser, hard code a filter in fuzzy.
We need to, and plan on, eventually rewriting the script. It would also make for a good student project, see here for a browsertime issue about this: https://github.com/sitespeedio/browsertime/issues/1681
If we integrate our existing tool, we'll only have less time in the future to rewrite the visualmetrics.py script. The existing implementation/methodology allows us to easily separate failures and decrease the possibility of data loss.
Assignee | ||
Comment 3•3 years ago
|
||
I've started implementing the potential quick solution to see how it looks. At this stage, it will look very similar to the visual-metrics implementation except for having multiple dependencies.
Comment 4•3 years ago
|
||
I think starting looking into the quick solution while plan for the long term one would be the best approach to unblock us and avoid high cost maintenance. So +1 for the choice.
Assignee | ||
Comment 5•3 years ago
|
||
I've implemented the quick solution yesterday actually, it needs a little more work but I'm able to get a task that depends on the test and vismet tasks and pulls their perfherder-data.json files: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=a10qh3ErQJKA2KjvK81nSA.0&revision=45931289adfc990af1c8bee4e1e6351496b1e3a7
Assignee | ||
Comment 6•3 years ago
|
||
At the moment, I am looking into whether or not we could integrate our visual-metrics script/calculations into the profiler as it already gathers screenshots and we'd be able to to run this in low-overhead mode too. We'll have the low-overhead profiling running in all of our tests in the future, so the screenshots will already be available to use for calculations within the same task as well.
Updated•3 years ago
|
Assignee | ||
Comment 7•3 years ago
|
||
EDIT: I didn't notice that I didn't update this bug to mention that I've decided to rework the visualmetrics.py script and remove the Imagemagick dependency and replace it with pure python dependencies.
Currently, I have managed to get all metrics matched up with what they were previously. All platforms work, and all browsers work. CSI is not an exact match but it's close enough and we will do a visual inspection of the edges overlaid onto the original video to determine if we are satisfied with proceeding with the change. Here's a try run that contains visual metrics calculated during the test: https://treeherder.mozilla.org/jobs?repo=try&tier=1%2C2%2C3&revision=dffbdccdd2cb0b4d8c4b0dc5ce38b8d0995efd64
Furthermore, the browsertime-results.tgz archive in that try run contains the videos with the edges overlaid on them. If everything is good after the next perf strategy meeting, the plan is to:
- Formalize the changes and land as a new script called
visualmetrics-portable.py
in the main browsertime repo. - Update raptor to run visual metrics during the test.
- Remove the
-vismet
tasks and associated code. - Remove browsertime-specific code from the Taskcluster actions.
- Update the frontend to undo changes that were needed for the 2-task system.
We'll also upstream the portable script to webpagetest if they're interested.
Assignee | ||
Comment 8•3 years ago
|
||
Assignee | ||
Comment 9•3 years ago
|
||
The PR has landed, and we now have access to the portable script in browsertime through the option --visualMetricsPortable.
Assignee | ||
Comment 10•3 years ago
|
||
This patch adds the node-16 toolchain to CI for all platforms used by browsertime. Required to update to browsertime v15+.
Assignee | ||
Comment 11•3 years ago
|
||
This patch removes all the code related to the second task (the *-vismet
tasks) we were using for visual-metrics processing.
Depends on D142836
Assignee | ||
Comment 12•3 years ago
|
||
This patch runs the visual-metrics processing within the test task rather than the (deleted) -vismet
tasks. It also updates the browsertime version to v15. It requires an update to node16. This patch is possible because the browsertime update gives us the new visualmetrics-portable.py
script which doesn't require ImageMagick anymore (they've been converted to Python dependencies). We did this conversion here: https://github.com/sitespeedio/browsertime/pull/1741
Depends on D142837
Comment 13•3 years ago
|
||
Assignee | ||
Updated•3 years ago
|
Comment 14•3 years ago
|
||
bugherder |
Assignee | ||
Comment 15•3 years ago
|
||
This patch adds the ability to raptor-browsertime for downloading, and installing a node binary from our Taskcluster CI. This makes it possible to seamlessly use raptor-browsertime with node-16, while using node-12 for the rest of the mozilla-central tooling.
Depends on D142837
Assignee | ||
Comment 16•3 years ago
|
||
This patch modifies the mach browsertime file to use the new visualmetrics script and removes the usage, installation, and checks for ImageMagick.
Depends on D142838
Assignee | ||
Comment 18•3 years ago
|
||
This patch forces us to re-install browsertime if FFMPEG is not found in the cache. This fixes an issue where deleting the .mozbuild/browsertime
folder would go undetected and cause failures later when running browsertime.
Alongside these changes, the patch also enables detecting for FFMPEG on Linux, and Windows machines. It works in the same way as mac, but it also requires using browsertime_ffmpeg
since the os.environ
settings don't get transferred to the required environment.
Depends on D143603
Assignee | ||
Comment 19•3 years ago
|
||
This patch causes us to trigger a rebuild for the python environment on browsertime upgrades and installations. This is needed for now because we are moving to python-based dependencies for visual-metrics processing and that requires a rebuild on the first upgrade/installation.
See bug 1766112 for the removal of this code.
Depends on D144464
Comment 20•3 years ago
|
||
Comment 21•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/9e27ff167552
https://hg.mozilla.org/mozilla-central/rev/7abf6bf1bbb3
https://hg.mozilla.org/mozilla-central/rev/9ceeff8173e0
https://hg.mozilla.org/mozilla-central/rev/203cddc7b976
https://hg.mozilla.org/mozilla-central/rev/797ecdf941f5
https://hg.mozilla.org/mozilla-central/rev/7bbdcfc5d92d
Assignee | ||
Updated•3 years ago
|
Updated•2 years ago
|
Updated•1 year ago
|
Description
•